[
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.
