db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "A B (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-681) Eliminate the parser's rewriting of the abstract syntax tree for queries with GROUP BY and/or HAVING clauses
Date Wed, 14 Feb 2007 19:51:05 GMT

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


Mime
View raw message