Skip Menu |
 

This queue is for tickets about the SQL-Statement CPAN distribution.

Report information
The Basics
Id: 69573
Status: stalled
Priority: 0/
Queue: SQL-Statement

People
Owner: Nobody in particular
Requestors: BBYRD [...] cpan.org
PRIVI [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: 1.33
Fixed in: (no value)



Subject: Multiple joins not supported?
Download (untitled) / with headers
text/plain 1.1k
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?
Download (untitled) / with headers
text/plain 334b
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.
Download (untitled) / with headers
text/plain 308b
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
Subject: Parser doesn't like parens in ON clause
Download (untitled) / with headers
text/plain 477b
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?
Download (untitled) / with headers
text/plain 121b
I have the test cases on my work laptop already. I'll submit a quick patch with that and some documentation adjustments.
Download (untitled) / with headers
text/plain 251b
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.)?
Download (untitled) / with headers
text/plain 864b
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)
Download (untitled) / with headers
text/plain 155b
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).
Download (untitled) / with headers
text/plain 108b
I put it into this because of it's all the same reason. Hope it's gonna fixed when rewriting SQL::Statement.
Download (untitled) / with headers
text/plain 1.2k
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.
RT-Send-CC: dbi-dev [...] perl.org
Download (untitled) / with headers
text/plain 2.7k
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).


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.