You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Rick Hillegas (JIRA)" <de...@db.apache.org> on 2005/11/03 21:53:44 UTC
[jira] Created: (DERBY-681) Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
------------------------------------------------------------------------------------------------------------
Key: DERBY-681
URL: http://issues.apache.org/jira/browse/DERBY-681
Project: Derby
Type: Bug
Reporter: Rick Hillegas
If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12710557#action_12710557 ]
Kathey Marsden commented on DERBY-681:
--------------------------------------
DERBY-4230 is a regression related to this issue. If the links are correct, this is our 12th regression related to this issue. The reports have trickled in over the past two years and have had a wide variety of symptoms.
I wonder if anyone has any ideas for a testing plan that we might be able to use to pop any additional issues related to this change?
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480548 ]
A B commented on DERBY-681:
---------------------------
> Do you think it makes sense to mark this bug as resolved and another to track this issue. Either way is fine.
I agree, either way is fine; I'll let you decide :) If you create a new issue, please mark it as a "regression" so that we can make sure it gets fixed before the next release. Otherwise it might slip through the cracks...
Thank you very much for your time with this...
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=all ]
Manish Khettry updated DERBY-681:
---------------------------------
Attachment: 681.patch.txt
This patch removes the "wrap group by's in a subselect" rewrite in the parser. It preserves the having clause through bind and optimize phases and during the final rewrite for aggregates in the GroupByNode, transforms the having clause to a valid restriction. I am also attaching a text file which should clarify the changes.
This patch also fixes related bugs DERBY-1624, the regresssion introduced by DERBY-280 and also completes the functionality for DERBY-883.
If this patch is approved and comitted I will file another bug to remove references to the flags generatedForGroupBy and geneatedForHaving which is dead code post this patch. I did not remove these references because it would make the patch even bigger and more complicated.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12479436 ]
A B commented on DERBY-681:
---------------------------
Thank you for the second patch, Manish. I looked it over and it does indeed address most of my comments, so I think we're getting closer. The reason I say "closer" is because I noticed the following problems with the second patch, and I think these should be addressed before the patch can be committed:
- Looks like the formatting for the first couple of lines in the license
text for views.sql has changed. I assume this was just an accident?
- This patch adds an explicit connection to views.sql, which I do not
think is right:
@@ -17,6 +14,7 @@
-- tests for views
-- set autocommit off
+connect 'jdbc:derby:wombat';
autocommit off;
Notice that the result (in views.out) is a security access violation:
@@ -17,7 +14,9 @@
-- tests for views
-- set autocommit off
-autocommit off;
+connect 'jdbc:derby:wombat';
+JAVA ERROR: java.security.AccessControlException: access denied (java.util.PropertyPermission ij.URLCheck read)
+ij> autocommit off;
This looks wrong to me. Is the additional connect statement an intentional change,
or is that just a remnant from some testing you were doing? If possible I think the
extra call to "connect" should be removed and the master file updated accordingly.
If you made the change intentionally then can you explain why it is necessary?
- There is also the following diff in the RunTest and RunSuite classes:
- classpath = sp.getProperty("classpath");
+ classpath = sp.getProperty("java.class.path");
Again, can you explain why this particular change is necessary? Were you
experiencing problems while running the tests? If so, what were those problems?
Other than these test issues I think the patch is ready for commit...
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Andrew McIntyre (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=comments#action_12454838 ]
Andrew McIntyre commented on DERBY-681:
---------------------------------------
Hi Manish,
I took a look at the current code for group by a while ago in relation to DERBY-1624 and eventually gave up as correctly fixing the issue went beyond my knowledge of the parsing/compilation phase of query processing and my available time. I've come across blog posts like this one:
http://blog.tremend.ro/2006/10/03/about-the-maturity-of-apache-derby/
in which the main difficulty was the inability to reference column aliases from the select list in group by clauses, so I think this is a worthwhile project to work on.
I came up with a band-aid of a fix which addressed DERBY-1624 by copying the relevant base column info and correlation name for the aliased columns in the rewritten subquery into the ColumnReferences for the result column list of the inner subquery, for which the correlation name could then be compared with the names of the result columns in the outer select with the group by clause to resolve the correct base column. IIRC, this approach worked for some cases, but there were failures in lang/groupby.sql that I didn't have time to track down.
I think a proper fix, as mentioned in the description of this issue, involves eliminating the rewriting of the query that happens in sqlgrammar.jj. If for some reason it might help you, though, I could try and find the patch for DERBY-1624 that I had worked on earlier.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Closed: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Rick Hillegas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rick Hillegas closed DERBY-681.
-------------------------------
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Myrna van Lunteren (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Myrna van Lunteren resolved DERBY-681.
--------------------------------------
Resolution: Fixed
Fix Version/s: 10.3.0.0
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Manish Khettry updated DERBY-681:
---------------------------------
Attachment: followup.patch.txt
query with a having clause but without a group by has a generated aggregaegt added on. I made visibleSize() less restrictive-- i.e. it takes into accounts all generated columns not just those added in GroupByList#bindGroupByColumns.
Another follow up item for me to look at is the different mechanism used for order by columns which are added to the select list. This is completely different from how group by does things.
I ran junit-all and derbylang.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Updated: (DERBY-681) Eliminate the parser's rewriting
of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by Army <qo...@gmail.com>.
Knut Anders Hatlen wrote:
>>A B updated DERBY-681:
>>----------------------
>>
>>I ran derbyall with the latest patch and there was one unexpected failure
>> (Derby client only):
>>
>>*** Start: setTransactionIsolation jdk1.4.2 DerbyNetClient
[snip]
>>However, I re-ran the test without your changes and it still failed,
>>so I do not think it's related. The machine on which I ran derbyall
>>is one I haven't used before so I don't know the specs; I'll look
>>into it.
>
> I think this problem may go away if you run ant clean or ant
> clobber. The canon for DerbyNetClient was removed about a week ago,
> but ant all won't automatically delete the old canon from classes.
Thanks for the reply, Knut Anders. I think that was indeed the problem.
Army
Re: [jira] Updated: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by Knut Anders Hatlen <Kn...@Sun.COM>.
"A B (JIRA)" <ji...@apache.org> writes:
> [ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
> A B updated DERBY-681:
> ----------------------
>
> Derby Info: (was: [Patch Available])
>
> I ran derbyall with the latest patch and there was one unexpected failure (Derby client only):
>
> *** Start: setTransactionIsolation jdk1.4.2 DerbyNetClient derbynetmats:jdbcapi 2007-03-08 18:28:42 ***
> 532 del
> < count=2, setTransactionIsolation() commits
> 532 add
>> passCommitCheck=true
> Test Failed.
>
> However, I re-ran the test without your changes and it still failed,
> so I do not think it's related. The machine on which I ran derbyall
> is one I haven't used before so I don't know the specs; I'll look
> into it.
I think this problem may go away if you run ant clean or ant
clobber. The canon for DerbyNetClient was removed about a week ago,
but ant all won't automatically delete the old canon from classes.
--
Knut Anders
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
A B updated DERBY-681:
----------------------
Derby Info: (was: [Patch Available])
I ran derbyall with the latest patch and there was one unexpected failure (Derby client only):
*** Start: setTransactionIsolation jdk1.4.2 DerbyNetClient derbynetmats:jdbcapi 2007-03-08 18:28:42 ***
532 del
< count=2, setTransactionIsolation() commits
532 add
> passCommitCheck=true
Test Failed.
However, I re-ran the test without your changes and it still failed, so I do not think it's related. The machine on which I ran derbyall is one I haven't used before so I don't know the specs; I'll look into it. In the meantime, I went ahead and committed 681_patch3.txt with svn # 516454:
URL: http://svn.apache.org/viewvc?view=rev&rev=516454
Thank you for your patience throughout this process, Manish!
As a reminder for future patches: the more involved you are in the community as a whole--especially in terms of reviewing other people's patches/specs--the more likely it is that people will in turn respond to comments/patches you yourself post, and probably in a much more timely manner. That's no guarantee, of course (this is after all open source), but it certainly doesn't hurt ;)
In any event, thanks again for the contribution!
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Manish Khettry resolved DERBY-681.
----------------------------------
Resolution: Fixed
This commit by Army, http://svn.apache.org/viewvc?view=rev&rev=518687, resolves issue.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480221 ]
Manish Khettry commented on DERBY-681:
--------------------------------------
hmmm, thanks for catching all of these. I should have thought about them and yes, it lis the correct fix.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472439 ]
Manish Khettry commented on DERBY-681:
--------------------------------------
Thanks for reviewing the patch. It will take me sometime to make the patch current and look at your comments. It has, after all, been a while since I submitted the patch.
I am curious-- is it typical for a patch to gather dust for a few months before someone finds the time to look at it? And if so, is this a good thing?
Thanks,
Manish
"A B (JIRA)" <ji...@apache.org> wrote:
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12471852 ]
A B commented on DERBY-681:
---------------------------
I noticed that this issue is now on the bottom of the "patch available" list--and that the patch was posted almost two months ago. So I did a quick review and my comments are below. Note: The patch is out of date (not surprisingly); it would be great if a newer version could be re-attached that incorporates the following comments (I am willing to continue the review/discussion until the patch is committed).
I think the difference between a fetch size of 1 and a fetch size of 16 comes down to the difference between a TableScan and BulkTableScan. I did a quick search of the codeline and it looks like Derby will disable bulk fetching if the result set is "ordering dependent" (see SelectNode.genProjectRestrict()) and also for certain min/max optimizations in a group by (see the "considerPostOptimizeOptimization()" method of GroupByNode). My guess is that your changes have somehow made it so that we no longer need to disable bulk fetch for certain queries, and thus you are now seeing a different fetch size. This seems particular likely since all of the queries that show a 1 vs 16 difference in aggregateOptimization() come under the heading of "group by ordered on grouping columns". So long story short, my feeling is that this is an acceptable diff...
Hopefully someone will correct me if I'm wrong...
Also (w.r.t to "notes.txt" as attached to this issue):
-- The notes you wrote for "Background on Group By" would be great as
javadoc comments in the GroupByNode.addNewColumnsForAggregation() method
(in addition to what's already there).
-- The notes you wrote for "Having clause -> Design" would be great as
comments in the GroupByNode.addAggregateColumns() method (perhaps just
before the "if (havingClause != null)"...)
Other review comments (note: I haven't done much actual testing yet, I've just looked at the code changes; I hope to do more testing of the changes next week...):
I think it would good if the following issues could be addressed before commit:
1) FromBaseTable.java:
-- It looks like you added an "accept()" method to FromBaseTable that
overrides ResultSetNode.accept(). I noticed that ResultSetNode.accept()
recursively calls "accept" on "resultColumns", but the new method in
FromBaseTable does not. This means that in cases where we used to accept
visitors for base table result columns we will no longer do so. I don't
know what the effects of that might be, but I think that's probably not
good. It would perhaps be better to call "super.accept()" at the start
of FromBaseTable.accept() and then go from there.
2) GroupByNode.java:
-- init() method has some lines that are commented out. You mention
these in your "notes.txt" file but there is no explanation as to why
they are commented out (and not just deleted) in the file itself;
might be good to add such comments (you could just take them from
notes.txt).
3) ResultColumnList.java:
-- With the following diff:
- int size = (size() > sourceRCL.size()) ? size() : sourceRCL.size();
+// int size = (size() > sourceRCL.size()) ? size() : sourceRCL.size();
+ int size = Math.min(size(), sourceRCL.size());
The original line (which is now commented out) looks like it assigns size
to be the *max* of size() and sourceRCL.size(); but your changes make it
use the minimum. I looked at the code and I think your fix is correct--
i.e. that we should be using the minimum size. So should the other line
just be deleted (instead of commented out)? Also, there appears to be an
implicit assumption that if the lists are two different sizes then the
shorter one must correspond to the _leading_ columns of the longer one.
If you're so inclined it might be nice to add a comment saying as much
(to go along with your change here).
4) ColumnReference.java:
-- Unrelated (and perhaps accidental) code cleanup diff; better to leave
this out of the patch.
5) GroupByExpressionTest.java:
-- In the "testSubSelect()" method of you added a test case that is
identical to the one immediately preceding it (so far as I can tell).
Was that intentional, or is there a HAVING clause missing?
The rest are nit-pick issues that I hope you might consider, though they should not block commit of the patch:
6) SelectNode.java:
-- Some unrelated code cleanup; not a big deal, but as a general rule
it detracts from the patch (makes it hard to figure out what changes
are related to the issue and what changes are not). Ex. See the
bindNonVTITables() method.
-- I wonder if we really need a new "init()" method here? As far as I
can tell there are only two files that currently create a SELECT node:
DeleteNode.java and sqlgrammar.jj. The latter uses the new init()
method, the former uses the old one--but the only difference is the
presence of the "havingClause" argument. DeleteNode already passes
in several null values (with associated comments), so would it make
sense to just add a null for "havingClause", as well? Then we would
only need the one "init()" method for SelectNode. I admit, though,
that this a stylistic detail and the code as you have it is correct.
So if you prefer to leave it as it is, that's fine--although it might
might be good to add some minimal javadoc to the new init() method
(ex. "@param havingClause The HAVING clause, if any", to keep in line
with the old init() method).
7) GroupByNode.java:
-- Unnecessary import of JBitSet and Properties.
-- Javadoc for "init()" method doesn't mention "nestingLevel"
-- Looks like the following comment was deleted:
- // finally reset gbc to its new position.
Was that intentional?
8) GroupByList.java:
-- Might be nice if you could add some comments for the following line:
+ selectRCL.setCountMismatchAllowed(true);
i.e. maybe explain briefly why count mismatches are allowed for GroupBy?
9) ResultColumnList.java:
-- With your changes there is now the following "if" statement in "propagateDCLInfo()":
/* Do both lists, if supplied by user, have the same degree? */
if (derivedRCL.size() != size() &&
! derivedRCL.getCountMismatchAllowed())
{
if (visibleSize() != derivedRCL.size()) {
throw StandardException.newException(SQLState.LANG_DERIVED_COLUMN_LIST_MISMATCH, tableName);
}
}
Could this be rewritten to:
/* Do both lists, if supplied by user, have the same degree? */
if (derivedRCL.size() != visibleSize() &&
! derivedRCL.getCountMismatchAllowed())
{
throw StandardException.newException(SQLState.LANG_DERIVED_COLUMN_LIST_MISMATCH, tableName);
}
As I said I haven't looked much at the new test cases nor have I done much testing myself. I will hopefully have time to do more of that next week...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------
TV dinner still cooling?
Check out "Tonight's Picks" on Yahoo! TV.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480437 ]
A B commented on DERBY-681:
---------------------------
Actually, it turns out that just switching to "visibleSize()" does *not* solve the problem. I made that change to a clean codeline and the error still occurs. When I printed out the values of "size()" and "visibleSize()", they *both* showed "2".
I don't know if it's relevant, but if you add a group by clause to the query, it runs fine:
select distinct vc, x from t1 as myt1
where d <= (select max(myt1.d) from t1 as myt2
where myt1.vc = myt2.vc and myt1.d <= myt2.d
group by d
having count(distinct d) <= 3);
Manish, would you be willing to look into this a bit more?
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kathey Marsden updated DERBY-681:
---------------------------------
Derby Info: (was: [Regression])
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kathey Marsden updated DERBY-681:
---------------------------------
Comment: was deleted
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Andrew McIntyre (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473226 ]
Andrew McIntyre commented on DERBY-681:
---------------------------------------
I checked Manish's patch and it does fix the original query reported not working in DERBY-1624. I applied Manish's patch and ran the original query in the description of DERBY-1624, and it works as expected.
While it would be great if column aliasing worked completely as expected with all of the queries in derby1624_repro.sql, I would argue that with Manish's patch committed DERBY-681 and DERBY-1624 should be closed, and a new issue opened to cover the remaining problems with column aliasing shown in the derby1624_repro script.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=all ]
Manish Khettry updated DERBY-681:
---------------------------------
Attachment: notes.txt
Notes on this patch.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Rick Hillegas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12710725#action_12710725 ]
Rick Hillegas commented on DERBY-681:
-------------------------------------
Hi Kathey,
I don't have any smart ideas, short of borrowing a query generator from some other project, generating a lot of queries (not just ones involving aggregates and GROUP BY), and comparing the results between Derby and some other database. Eliminating the parser's rewriting of the AST was a significant change to Derby's SQL interpreter. 8 years of code had piled up on top of the assumption that the AST was rewritten in the parser. The assumptions weren't linked to one another in any systematic way and I don't know of any systematic way to track them all down. So far, we have seen bugs related to GROUP BY, sort order, subqueries, unions, and column aliasing. The bugs are peppered all across the language.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kathey Marsden updated DERBY-681:
---------------------------------
Derby Info: [Regression]
This is a regression caused by DERBY-681 (svn revision 516454)
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Manish Khettry updated DERBY-681:
---------------------------------
Attachment: 681.patch3.txt
removed runtest, runsuite. fixed views.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Manish Khettry updated DERBY-681:
---------------------------------
Attachment: 681.patch2.txt
Again, thanks for very detailed and constructive review and for finding the problems with union/views/create table. I ran derbylang (one failure compressTable once, DERBY-2117?) and junit-all (one failure in a derbynet tearDown method?). Normally I would try to track this failure but this bugfix has been sitting on my laptop for too long and it really doesn't look like anything to do with my changes (famous last words?!)
I have accepted most of the review comments from A. B atleast partially except for 4, 6 and 9.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=comments#action_12455195 ]
Manish Khettry commented on DERBY-681:
--------------------------------------
Thanks Andrew. I hope that I can eliminate the "group/having" query rewrite and this should fix DERBY-1624.
On the blog entry-- I do think that being able to use aliases in the query (group by or where clause) would be helpful. I remember seeing a bug about this issue a while ago.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Satheesh Bandaram (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=comments#action_12356724 ]
Satheesh Bandaram commented on DERBY-681:
-----------------------------------------
Should this be marked an Enhancement request? While I agree the current scheme of rewriting the query can be improved, it does achieve the functionality. Bugs in the current implementation have been logged separately. (like DERBY-280) Changing the mechanism should be marked an Enhancement, I think.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Type: Improvement
> Reporter: Rick Hillegas
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12739710#action_12739710 ]
Kathey Marsden commented on DERBY-681:
--------------------------------------
As a matter of bookkeeping, if all of the wisconsin diffs are deemed acceptable and other tests and reviews pass, I think we should do the partial backout as a resolution of this issue. Then both DERBY-3926 and DERBY-4331 can be resolved and a new issue opened for any follow up sort avoidance optimizations.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480501 ]
Manish Khettry commented on DERBY-681:
--------------------------------------
OK. Do you think it makes sense to mark this bug as resolved and another to track this issue. Either way is fine.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12471852 ]
A B commented on DERBY-681:
---------------------------
I noticed that this issue is now on the bottom of the "patch available" list--and that the patch was posted almost two months ago. So I did a quick review and my comments are below. Note: The patch is out of date (not surprisingly); it would be great if a newer version could be re-attached that incorporates the following comments (I am willing to continue the review/discussion until the patch is committed).
Oh, and thank you for the "notes.txt" file, Manish! I found it to be very helpful as I went through the code changes. In that document you wrote (at the very end):
> Also the one thing that I cannot explain in this patch: the fetch size
> in some ResultSet nodes has gone from 1 to 16. If this is serious I can
> dig in to see why this is happening.
I think the difference between a fetch size of 1 and a fetch size of 16 comes down to the difference between a TableScan and BulkTableScan. I did a quick search of the codeline and it looks like Derby will disable bulk fetching if the result set is "ordering dependent" (see SelectNode.genProjectRestrict()) and also for certain min/max optimizations in a group by (see the "considerPostOptimizeOptimization()" method of GroupByNode). My guess is that your changes have somehow made it so that we no longer need to disable bulk fetch for certain queries, and thus you are now seeing a different fetch size. This seems particular likely since all of the queries that show a 1 vs 16 difference in aggregateOptimization() come under the heading of "group by ordered on grouping columns". So long story short, my feeling is that this is an acceptable diff...
Hopefully someone will correct me if I'm wrong...
Also (w.r.t to "notes.txt" as attached to this issue):
-- The notes you wrote for "Background on Group By" would be great as
javadoc comments in the GroupByNode.addNewColumnsForAggregation() method
(in addition to what's already there).
-- The notes you wrote for "Having clause -> Design" would be great as
comments in the GroupByNode.addAggregateColumns() method (perhaps just
before the "if (havingClause != null)"...)
Other review comments (note: I haven't done much actual testing yet, I've just looked at the code changes; I hope to do more testing of the changes next week...):
I think it would good if the following issues could be addressed before commit:
1) FromBaseTable.java:
-- It looks like you added an "accept()" method to FromBaseTable that
overrides ResultSetNode.accept(). I noticed that ResultSetNode.accept()
recursively calls "accept" on "resultColumns", but the new method in
FromBaseTable does not. This means that in cases where we used to accept
visitors for base table result columns we will no longer do so. I don't
know what the effects of that might be, but I think that's probably not
good. It would perhaps be better to call "super.accept()" at the start
of FromBaseTable.accept() and then go from there.
2) GroupByNode.java:
-- init() method has some lines that are commented out. You mention
these in your "notes.txt" file but there is no explanation as to why
they are commented out (and not just deleted) in the file itself;
might be good to add such comments (you could just take them from
notes.txt).
3) ResultColumnList.java:
-- With the following diff:
- int size = (size() > sourceRCL.size()) ? size() : sourceRCL.size();
+// int size = (size() > sourceRCL.size()) ? size() : sourceRCL.size();
+ int size = Math.min(size(), sourceRCL.size());
The original line (which is now commented out) looks like it assigns size
to be the *max* of size() and sourceRCL.size(); but your changes make it
use the minimum. I looked at the code and I think your fix is correct--
i.e. that we should be using the minimum size. So should the other line
just be deleted (instead of commented out)? Also, there appears to be an
implicit assumption that if the lists are two different sizes then the
shorter one must correspond to the _leading_ columns of the longer one.
If you're so inclined it might be nice to add a comment saying as much
(to go along with your change here).
4) ColumnReference.java:
-- Unrelated (and perhaps accidental) code cleanup diff; better to leave
this out of the patch.
5) GroupByExpressionTest.java:
-- In the "testSubSelect()" method of you added a test case that is
identical to the one immediately preceding it (so far as I can tell).
Was that intentional, or is there a HAVING clause missing?
The rest are nit-pick issues that I hope you might consider, though they should not block commit of the patch:
6) SelectNode.java:
-- Some unrelated code cleanup; not a big deal, but as a general rule
it detracts from the patch (makes it hard to figure out what changes
are related to the issue and what changes are not). Ex. See the
bindNonVTITables() method.
-- I wonder if we really need a new "init()" method here? As far as I
can tell there are only two files that currently create a SELECT node:
DeleteNode.java and sqlgrammar.jj. The latter uses the new init()
method, the former uses the old one--but the only difference is the
presence of the "havingClause" argument. DeleteNode already passes
in several null values (with associated comments), so would it make
sense to just add a null for "havingClause", as well? Then we would
only need the one "init()" method for SelectNode. I admit, though,
that this a stylistic detail and the code as you have it is correct.
So if you prefer to leave it as it is, that's fine--although it might
might be good to add some minimal javadoc to the new init() method
(ex. "@param havingClause The HAVING clause, if any", to keep in line
with the old init() method).
7) GroupByNode.java:
-- Unnecessary import of JBitSet and Properties.
-- Javadoc for "init()" method doesn't mention "nestingLevel"
-- Looks like the following comment was deleted:
- // finally reset gbc to its new position.
Was that intentional?
8) GroupByList.java:
-- Might be nice if you could add some comments for the following line:
+ selectRCL.setCountMismatchAllowed(true);
i.e. maybe explain briefly why count mismatches are allowed for GroupBy?
9) ResultColumnList.java:
-- With your changes there is now the following "if" statement in "propagateDCLInfo()":
/* Do both lists, if supplied by user, have the same degree? */
if (derivedRCL.size() != size() &&
! derivedRCL.getCountMismatchAllowed())
{
if (visibleSize() != derivedRCL.size()) {
throw StandardException.newException(SQLState.LANG_DERIVED_COLUMN_LIST_MISMATCH, tableName);
}
}
Could this be rewritten to:
/* Do both lists, if supplied by user, have the same degree? */
if (derivedRCL.size() != visibleSize() &&
! derivedRCL.getCountMismatchAllowed())
{
throw StandardException.newException(SQLState.LANG_DERIVED_COLUMN_LIST_MISMATCH, tableName);
}
As I said I haven't looked much at the new test cases nor have I done much testing myself. I will hopefully have time to do more of that next week...
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=all ]
Manish Khettry updated DERBY-681:
---------------------------------
Derby Info: [Patch Available]
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Issue Comment Edited: (DERBY-681) Eliminate the parser's
rewriting of the abstract syntax tree for queries with GROUP BY and/or
HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12547980 ]
army edited comment on DERBY-681 at 12/3/07 12:32 PM:
-----------------------------------------------------
Looks like the fix for this issue caused a sorting regression as described in DERBY-3231. Manish, are you available to look into that a little more?
was (Author: army):
Looks like the fix for this issue caused a sorting regression, per DERBY-3231. Manish, are you available to look into that a little more?
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (DERBY-681) Eliminate the parser's rewriting
of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by Daniel John Debrunner <dj...@apache.org>.
A B (JIRA) wrote:
> [ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472754 ]
>
> A B commented on DERBY-681:
> ---------------------------
>
> Manish Khettry (JIRA) wrote:
>> Thanks for reviewing the patch. It will take me sometime to make
>> the patch current and look at your comments. It has, after all,
>> been a while since I submitted the patch.
>
> Thank you for willingness to continue working with the patch.
>
>> I am curious-- is it typical for a patch to gather dust for a
>> few months before someone finds the time to look at it?
>
> I don't think it's "typical" for this to happen, no. But given the "fry your own fish" philosophy of open source development, this kind of thing does (unfortunately) happen on occasion.
>
>
>That said, though, it's up to people in the community (users, developers, anyone--doesn't just have to be committers)
>to review patches and/or comment on the various issues according to
their own interest/expertise.
And if anyone is concerned about such delays they can help by reviewing
patches that they didn't submit, possibly allowing a committer just to
perform the actual commit. Then hopefully someone else will spend the
effort to review their patch.
Community is a key word here, it takes people being active to make a
community. There is not a fixed set of folks working on Derby who are
responsible for the project, the community is, and that's only exists
when people participate and interact. People frying their own fish is
perfectly acceptable, but if that's all everyone does then we don't have
a community.
Dan.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472754 ]
A B commented on DERBY-681:
---------------------------
Manish Khettry (JIRA) wrote:
>
> Thanks for reviewing the patch. It will take me sometime to make
> the patch current and look at your comments. It has, after all,
> been a while since I submitted the patch.
Thank you for willingness to continue working with the patch.
> I am curious-- is it typical for a patch to gather dust for a
> few months before someone finds the time to look at it?
I don't think it's "typical" for this to happen, no. But given the "fry your own fish" philosophy of open source development, this kind of thing does (unfortunately) happen on occasion.
The derby-dev mailing list receives a daily "subscription" email which lists issues having the "Patch Available" flag set. The issues are sorted according to the date of that last update/comment; those at the bottom of the list are the ones that have been sitting the longest with no activity. Ex.:
http://thread.gmane.org/gmane.comp.apache.db.derby.devel/36563
That said, though, it's up to people in the community (users, developers, anyone--doesn't just have to be committers) to review patches and/or comment on the various issues according to their own interest/expertise. And as you've seen, sometimes the result is that things slip through the cracks.
I noticed DERBY-681 at the bottom of the list last week, which is what prompted me to look it up--and in doing so I saw that it had been idle for two months(!). So as I mentioned in my previous comment, I am willing to work with you on this patch so that it can now be committed. Apologies for the awfully long delay...
It's definitely not a good thing to have patches sitting for so long without review. While the decision as to who reviews what patches is entirely up to those in the community, you should feel free to "push" your patch if it's not getting any attention. See "Extra Mile" advice on the wiki:
http://wiki.apache.org/db-derby/PatchAdvice
And in particular, note the following quote:
"Post to the list at least every couple of weeks if you don't get review. Ask if someone is available to look at your patch and what additional information reviewers might need for review."
I have found that sending an explicit email to derby-dev asking for people to review a patch which has been idle for a couple of weeks almost always leads to a reply and a subsequent review. So feel free to do so!
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473218 ]
Manish Khettry commented on DERBY-681:
--------------------------------------
Just a quick note to address two issues raised by Army.
DERBY-1624: It won't fix all the queries attached by Andrew for this bug because we don't resolve aliases in the group by/having/orderby clauses; the following query will fail. This bug does not address aliases-- only the original query with this bug.
select x as alias, sum(y)
from t
group by alias;
The size vs visible size is more problematic-- I really think the language layer should not be using RCL directly as much as it currently does. If it wants to check the number of columns returned by a ResultSetNode, it should call a method in the RSN called either visibleSize or runtimeSize or some such (i.e. the # of columns actually returned by the result set node). The only way to find all of these would be to grind through all the occurences of RCL.size! This in itself should suggest that the use of these classes/methods in this little area itself (i.e. # of columns in a result set node) could deal with a good dose of refactoring.
Thanks for looking at the patch, running tests and asking the right questions. I appreciate the time you've taken for this patch. I am a little busy at work but will get back to this bug when things let up (hopefully next week).
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kathey Marsden updated DERBY-681:
---------------------------------
Comment: was deleted
(was: As a matter of bookkeeping, if all of the wisconsin diffs are deemed acceptable and other tests and reviews pass, I think we should do the partial backout as a resolution of this issue. Then both DERBY-3926 and DERBY-4331 can be resolved and a new issue opened for any follow up sort avoidance optimizations.
)
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12481222 ]
A B commented on DERBY-681:
---------------------------
Thank you for the follow-up patch, Manish. I verified that "followup.patch.txt" solves the problem I reported above, so I committed the patch after adding a corresponding test case to lang/subquery.sql (always good to have a regression test case when possible):
URL: http://svn.apache.org/viewvc?view=rev&rev=518687
I assume that the numGeneratedColumnsForGroupBy() method, which is now no longer in use, will be removed as part of your work for DERBY-2442?
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Myrna van Lunteren (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Myrna van Lunteren reopened DERBY-681:
--------------------------------------
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=comments#action_12452071 ]
Manish Khettry commented on DERBY-681:
--------------------------------------
I was looking at the query tree munging that we do in the GroupByNode and it seems that doing something with the having clause should be possible. Basically we always put a ProjectRestrictNode on top of a GroupByNode. It should be possible to rewrite the having clause and turn it into a restriction of this ProjectRestrictNode. This rewriting of the query tree should be pretty similar to DERBY-883.
Is this what you mean by "handle the having clause in a starightforward way"? If anyone has thought about this bug and have any ideas on this, please do share. I can work on this next-- it should help with atleast a couple of other bugs (DERBY-280 and DERBY-1624) but wanted to check with others on the list before I start working on it.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Assigned: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Manish Khettry (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=all ]
Manish Khettry reassigned DERBY-681:
------------------------------------
Assignee: Manish Khettry
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Donatas Ciuksys (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12557159#action_12557159 ]
Donatas Ciuksys commented on DERBY-681:
---------------------------------------
DERBY-3303 can also be observed on derby 10.3.1.4, so the bug might have crept in earlier.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the
abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Mike Matrigali (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=all ]
Mike Matrigali updated DERBY-681:
---------------------------------
Component: SQL
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473169 ]
A B commented on DERBY-681:
---------------------------
I spent some time manually trying out different test cases with the patch for this issue and things seem to work well for the most part (see below). I ran the new test cases that this patch adds to GroupByExpressionTest and they behave as expected: i.e. they pass with the patch and fail without it.
I also ran the repro case for DERBY-280 and verified that Yes, the query still runs correctly with these changes. So that's good :)
Regarding "for most the part": I did find a couple of scenarios where d681.patch.txt causes queries to fail where they used to succeed. I found these while playing with the DDL from the queries attached to DERBY-1624, namely:
create table o (name varchar(20), ord int);
create table a (ord int, amount int);
With these tables and some minimal data, I noticed that the following statement, which used to work, fails with error 42X56 after applying d681.patch:
create view v1 (vx, vy) as
select name, sum(ord) from o where ord > 0 group by name, ord
having ord <= ANY (select ord from a);
I think the problem is that there is a check in CreateViewNode which uses the "size()" method of ResultColumnList instead of the new "visibleSize()" method. When I made that change the above statement executes as normal.
I then found another example that shows a similar problem:
create table ov (ox, oy) as
(select name, sum(ord) from o where ord > 0 group by ord, name
having ord <= ANY (select ord from a)) with no data;
I think the underlying problem is again the use of "size()" instead of "visibleSize()".
And if we assume the view "v1" from above is made to work (by changing the appropriate "size()" call to "visibleSize()") then the following statement, which does a union between v1 and a query on v1, will end up failing, too:
select vx, vy from v1
union select vx, sum(vy) from v1 group by vx, vy having (vy / 2) > 15;
So that's three examples where use of "size()" instead of "visibleSize()" is leading to an error--and each of the three errors has its origin in a different file. Which makes me wonder: are there other places where this same change needs to be made but we haven't found them yet? Is there a reliable way to search the codeline for this kind of thing? In any event, it would be good if the respective files for those three examples could be updated as necessary.
Aside from the above issue I couldn't see any other functional problems with the patch. Thank you very much for your explanations of the diffs in the various tests; that saved me some time :)
On an entirely different note, I noticed that when the patch was posted you included a comment saying:
"This patch also fixes related bugs DERBY-1624 [...]"
When I read that I assumed that the queries attached to DERBY-1624 would now pass with d681.patch.txt--but that does not appear to be the case. Of the 18 queries in "1624_repro.sql", 10 pass when I apply the patch for DERBY-681 (only 7 pass without your changes). So we're getting better. However, it looks like there is still more work to do before DERBY-1624 can be considered resolved. Do you agree?
I have not run derbyall nor suites.All yet as I'm waiting for an updated patch before doing so...
In the meantime, thank you again for taking the time to tackle this particular issue--your contribution and your patience are both much appreciated.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12710942#action_12710942 ]
Kathey Marsden commented on DERBY-681:
--------------------------------------
Well I drilled down on those linked issues and 12 was too harsh. Some of the linked issues were actually fixed by this change.
The idea of borrowing a query generator from some other project and comparing results with another db sounds like an interesting one for a new angle on general testing as well.
It looks like DERBY-4230 is an issue of size() being used where visibleSize() was appropriate. I seem to recall this being an issue with at least one other issue. It might be interesting to look at the other size() calls and make sure they shouldn't be visibleSize() instead now that the rewrite is gone.
A strange thing about Eclipse is when I search for references to QueryTreeNodeVector.size(), I am getting back all references to a size() method regardless of the class, which is a quite a lot of references.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Manish Khettry
> Fix For: 10.3.1.4
>
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, followup.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473232 ]
A B commented on DERBY-681:
---------------------------
> I would argue that with Manish's patch committed DERBY-681 and DERBY-1624 should be closed,
> and a new issue opened to cover the remaining problems with column aliasing shown in the
> derby1624_repro script.
Sounds good to me.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-681) Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-681?page=all ]
Daniel John Debrunner updated DERBY-681:
----------------------------------------
type: Improvement (was: Bug)
Seems more like an improvement than a bug fix.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: http://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Type: Improvement
> Reporter: Rick Hillegas
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-681) Eliminate the parser's rewriting of
the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12480197 ]
A B commented on DERBY-681:
---------------------------
Hi Manish,
I think I've found another case where use of "size()" instead of "visibleSize()" is leading to incorrect behavior after the patch for this issue. Take the following example:
ij> create table t1 (d double, vc varchar(20), x int);
0 rows inserted/updated/deleted
ij> select distinct vc, x from t1 as myt1
where d <= (select max(myt1.d) from t1 as myt2
where myt1.vc = myt2.vc and myt1.d <= myt2.d
having count(distinct d) <= 3);
ERROR 42X39: Subquery is only allowed to return a single column.
Before the commit for this issue the above query ran without error. But now, with the latest trunk, the query fails because Derby thinks the subquery is returning more than one column, which is not true. The relevant code is in SubqueryNode.java:
/* The parser does not enforce the fact that a subquery can only return
* a single column, so we must check here.
*/
if (resultColumns.size() != 1)
{
throw StandardException.newException(SQLState.LANG_NON_SINGLE_COLUMN_SUBQUERY);
}
I think the call to "size()" here should be replaced with a call to "visibleSize()". If you agree that this is the correct fix then I can go ahead and commit this change to trunk as a "followup" patch for this issue.
> Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
> ------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-681
> URL: https://issues.apache.org/jira/browse/DERBY-681
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assigned To: Manish Khettry
> Attachments: 681.patch.txt, 681.patch2.txt, 681.patch3.txt, notes.txt
>
>
> If a query contains a GROUP BY or HAVING clause, the parser rewrites the abstract syntax tree, putting aggregates into a subselect and treating the HAVING clause as the WHERE clause of a fabricated outer select from the subquery. This allows the compiler to re-use some machinery since the HAVING clause operates on the grouped result the way that the WHERE clause operates on the from list. Unfortunately, this rewriting creates an explosion of special cases in the compiler after parsing is done. The rewriting is not systematically handled later on in the compiler. This gives rise to defects like bug 280. We need to eliminate this special rewriting and handle the HAVING clause in a straightforward way. This is not a small bugfix but is a medium sized project.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.