Bug #69573 for SQL-Statement: Multiple joins not supported?

Tue Jul 19 06:54:35 2011 PRIVI [...] cpan.org - Ticket created

Subject:

Multiple joins not supported?

Hi SQL Statement developer, VERIONS: SQL::Statement 1.33 DBD::CSV 0.31 the SQL statement SELECT logfile.id, art.name FROM logfile LEFT JOIN art ON art.id=logfile.art_id works as expected. The next stamtement is executing but don't fetch any data from certs or art: SELECT logfile.id, logfile.art_id, art.name FROM logfile LEFT JOIN art ON art.id=logfile.art_id LEFT JOIN certs ON certs.id=logfile.certs_id . Here is only "logfile.id" defined, art.name is empty. The next one is also executing but don't hava value for certs or art: SELECT logfile.id, certs.id FROM logfile LEFT JOIN certs ON certs.id=logfile.certs_id LEFT JOIN art ON art.id=logfile.art_id The following statement SELECT logfile.id, art.name, certs.id FROM logfile LEFT JOIN art ON art.id=logfile.art_id LEFT JOIN certs ON certs.id=logfile.certs_id brings an unexpected error: Table 'certs' referenced in WHERE but not found in FROM list (art,logfile)! at /usr/local/share/perl/5.10.1/SQL/Statement.pm line 88 CONCLUSION ==>> It seems that joining multiple tables is just not supported on DBD::CSV, but SQL::Statement allows to request it in some cases with bad behavior?

Wed Jul 20 07:18:19 2011 PRIVI [...] cpan.org - Correspondence added

As https://rt.cpan.org/Ticket/Display.html?id=30547#txn-380451 documents, multiple joins are not supported. Only the error behaviour could still be improved. And maybe the documentation, I can't see any hint that it is not supported. Only the syntax could some people tell you that it don't work... I don't think that this is enough.

Wed Jul 20 07:18:21 2011 PRIVI [...] cpan.org - Status changed from 'new' to 'rejected'

Wed Jul 20 07:29:11 2011 REHSACK [...] cpan.org - Correspondence added

I agree, at least the documentation can be updated. I'll take a look whether it's possible to update the behaviour, but this cannot promised due limitations of the current implementation. If you want to help, please add some "TODO" tests to t/08join.t or send me a patch updating the documentation :) Jens

Wed Jul 20 07:29:12 2011 The RT System itself - Status changed from 'rejected' to 'open'

Wed Oct 19 12:06:30 2011 BBYRD [...] cpan.org - Ticket #71780: Ticket created

Subject:

Parser doesn't like parens in ON clause

use SQL::Statement; my $sql = " SELECT a, q FROM b fg INNER JOIN c asd ON (asd.q = fg.r) "; my $parser = SQL::Parser->new('ANSI'); $parser->{RaiseError}=1; $parser->{PrintError}=0; my $stmt = SQL::Statement->new($sql,$parser); This fails because of the parens. Not sure if it's legal ANSI SQL syntax, but it is legal in all of the other DBs I've used (MySQL, T-SQL, Oracle). What's the difficulty level in making the parser accept ON clauses in parens?

Mon Mar 19 22:17:33 2012 BBYRD [...] cpan.org - Correspondence added

I have the test cases on my work laptop already. I'll submit a quick patch with that and some documentation adjustments.

Tue Mar 20 08:51:15 2012 REHSACK [...] cpan.org - Correspondence added

Awesome. I'm interested to see them. Did you patch against subversion trunk or any released version? Do only t/08join.t works now or does make test in general work (with installed DBD::CSV, DBD::AnyData, DBD::SQLite (as reference), current DBI etc.)?

Wed Mar 21 12:04:03 2012 BBYRD [...] cpan.org - Correspondence added

On Tue Mar 20 08:51:15 2012, REHSACK wrote: Show quoted text

> Awesome. I'm interested to see them. Did you patch against subversion > trunk or any released version? > > Do only t/08join.t works now or does make test in general work (with > installed DBD::CSV, DBD::AnyData, DBD::SQLite (as reference), current > DBI etc.)?

Sorry, didn't mean to get your hopes up. I only meant that I have documentation specifying that multiple JOINs aren't supported, and TODO test cases. I'd like to get this fixed, but given that most of these RT bugs are parsing errors, it seems like the best route is going to be deployment of some sort of Bison/Flex parser. PG has the code, but only the scanner appears to be separable from the rest of the RDBMS backend. (The parser itself is too integrated, according to the PG team themselves.) Still, it might be a start to get things rolling...

Subject:

multi-join-doc.patch

Index: lib/SQL/Statement/Syntax.pod =================================================================== --- lib/SQL/Statement/Syntax.pod (revision 15224) +++ lib/SQL/Statement/Syntax.pod (working copy) @@ -232,6 +232,9 @@ * self-joins are not currently supported * if implicit joins are used, the WHERE clause must contain an equijoin condition for each table + * multiple ANSI joins are not supported; use implicit joins for these + * this also means that combinations of INNER and non-INNER joins are + not supported =head3 SEARCH CONDITION Index: t/08join.t =================================================================== --- t/08join.t (revision 15224) +++ t/08join.t (working copy) @@ -541,6 +541,48 @@ [ '[% NOW %]', 'cpan-authors', 'cpan-authors', '8.4.1', 'statler', ], ], }, + { + test => "Complex INNER JOIN", + sql => q{SELECT pname, sname, rname + FROM Prof p + JOIN Subject s + ON p.pid = s.pid + JOIN Room r + ON p.pid = r.pid + }, + result => [ [qw(Sue Chem 1C)], [qw(Bob Bio 2B)], [qw(Bob Math 2B)] ], + todo => 'Not supported yet!', + }, + { + test => "Complex INNER JOIN (using)", + sql => q{SELECT pname, sname, rname + FROM Prof + JOIN Subject USING (pid) + JOIN Room USING (pid) + }, + result => [ [qw(Sue Chem 1C)], [qw(Bob Bio 2B)], [qw(Bob Math 2B)] ], + todo => 'Not supported yet!', + }, + { + test => "Complex NATURAL JOIN", + sql => q{SELECT pname, sname, rname + FROM Prof NATURAL JOIN Subject NATURAL JOIN Room + }, + result => [ [qw(Sue Chem 1C)], [qw(Bob Bio 2B)], [qw(Bob Math 2B)] ], + todo => 'Not supported yet!', + }, + { + test => "Complex LEFT JOIN", + sql => q{SELECT pname, sname, rname + FROM Prof p + LEFT JOIN Subject s + ON p.pid = s.pid + LEFT JOIN Room r + ON p.pid = r.pid + }, + result => [ [qw(Sue Chem 1C)], [qw(Bob Bio 2B)], [qw(Bob Math 2B)], ['Tom', undef, undef] ], + todo => 'Not supported yet!', + }, ); foreach my $test (@tests)

Tue Oct 02 10:44:42 2012 REHSACK [...] cpan.org - Correspondence added

Patches applied with changes after failure analysing session. Can't believe that your version of t/08join.t run successful (from Test::More's perspective).

Tue Oct 02 10:44:43 2012 REHSACK [...] cpan.org - Status changed from 'open' to 'patched'

Tue Oct 02 10:56:09 2012 REHSACK [...] cpan.org - Ticket #71780: Correspondence added

I put it into this because of it's all the same reason. Hope it's gonna fixed when rewriting SQL::Statement.

Tue Oct 02 10:56:10 2012 The RT System itself - Ticket #71780: Status changed from 'new' to 'open'

Tue Oct 02 10:56:10 2012 REHSACK [...] cpan.org - Ticket #71780: Merged into ticket #69573

Tue Oct 02 10:56:10 2012 REHSACK [...] cpan.org - Merged into ticket #69573

Tue Oct 02 11:58:41 2012 BBYRD [...] cpan.org - Correspondence added

On Tue Oct 02 10:56:09 2012, REHSACK wrote: Show quoted text

> I put it into this because of it's all the same reason. Hope it's gonna > fixed when rewriting SQL::Statement.

Believe it or not, I am actively developing in this area, and have been for the last couple of months. Latest DFS levels: 1. SQL::Statement needs a new parser. Parser should be something like SQLT, with multiple in/out parsers. 2. SQLT currently does not work with non-schema statements, so a new parser project should be created. Parser should borrow from a real, active RDBMS like PostgreSQL. 3. Pg's parser is written in Bison/C. Need to convert to Perl. 4. Successfully converted to Eyapp, but Eyapp is pretty outdated and inefficient as a parser language. Queries still slow. Would have more success with a modern top-down parser like Pegex. 5. Pegex is a rather new language and needs some feature enchantments to help support the conversion and provide a long-term code base for the parser. Working with Ingy to shape language to work better with larger parsers like this. Currently at level 5, which I think is about as far down the rabbit hole as you can go. Have written a quick eyapp2pegex convertor, but like I said, some shaping of the language needs to happen first to keep things sane.

Tue Oct 02 11:58:42 2012 The RT System itself - Status changed from 'patched' to 'open'

Tue Oct 02 14:25:15 2012 REHSACK [...] cpan.org - Correspondence added

RT-Send-CC:

dbi-dev [...] perl.org

On Tue Oct 02 11:58:41 2012, BBYRD wrote: Show quoted text

> On Tue Oct 02 10:56:09 2012, REHSACK wrote:

> > I put it into this because of it's all the same reason. Hope it's gonna > > fixed when rewriting SQL::Statement.

> > Believe it or not, I am actively developing in this area, and have been > for the last couple of months.

I believe that. That was not a comment to your effort nor your skills - it was a comment on the quality of the patch. At first it didn't apply cleanly and second it leads "make test" to fail. Because I know that you active developing, I didn't simply reply "patched with modifications" - I told you it costs me a lot extra effort and hope your next submissions will be better. Show quoted text

> Latest DFS levels: > > 1. SQL::Statement needs a new parser. Parser should be something like > SQLT, with multiple in/out parsers.

Yes, we all agree on it. I talked to mst and ribasushi about that and I think we found a solution from several kind of views. When ribasushi is back from USA we meet again and talk about some details. Show quoted text

> 2. SQLT currently does not work with non-schema statements, so a new > parser project should be created. Parser should borrow from a real, > active RDBMS like PostgreSQL.

Additional ones might be - but the one being bundled with SQL::Statement will definitively not. But the design we discussed at YAPC::EU should allow you to write as many parsers supporting as many dialects as your projects require. This was a major goal I wanted to reach (because I understood your requirements - I just disagree fulfilling them in the SQL::Parser). Show quoted text

> 3. Pg's parser is written in Bison/C. Need to convert to Perl. > > 4. Successfully converted to Eyapp, but Eyapp is pretty outdated and > inefficient as a parser language. Queries still slow. Would have more > success with a modern top-down parser like Pegex.

I'm always open for suggestions. Pegex looks interesting, thanks for the suggestion. But to be true - a real good thing would be some kind of tool which is able to generate Pure-Perl Parser code as well as optional XS support without requiring additional modules at runtime. Show quoted text

> 5. Pegex is a rather new language and needs some feature enchantments to > help support the conversion and provide a long-term code base for the > parser. Working with Ingy to shape language to work better with larger > parsers like this. > > Currently at level 5, which I think is about as far down the rabbit hole > as you can go. Have written a quick eyapp2pegex convertor, but like I > said, some shaping of the language needs to happen first to keep things > sane.

Let's see what future brings. A token based parser is the next primary goal - this is the thing I agree on. I'd prefer to have that kind of discussion on dbi-dev@ (with probably ribasushi, frew and mst on CC - for Data::Query and SQLT parts of discussion).

Mon Oct 29 09:48:01 2012 REHSACK [...] cpan.org - Severity Wishlist added

Mon Oct 29 09:48:01 2012 REHSACK [...] cpan.org - Status changed from 'open' to 'stalled'