db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Nielsen (JIRA)" <j...@apache.org>
Subject [jira] Updated: (DERBY-2998) Add support for ROW_NUMBER() window function
Date Tue, 26 Feb 2008 11:55:52 GMT

     [ https://issues.apache.org/jira/browse/DERBY-2998?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel

Thomas Nielsen updated DERBY-2998:

    Attachment: d2998-16.stat

Thanks again for having a look Army!

Attaching patch d2998-16.diff that fixes most of Army comments, plus a few other very minor
issues. Testwise I get the same result with patch 16.

> 1)  ....  I wonder if it would be worth it to add a new "isWindowColumn()" method

Agree - makes the code more easily readable. Fixed in patch 16.

Also added RC.isVirtualWindowColumn() for checking for a VirtualColumnNode pointing to a WindowFunctionColumnNode.

> 2)
> It might be good to add an ASSERT here to make sure that the 'hopefully' condition is

The comment is not correct and should be removed. Fixed in patch 16 by rephrasing to reflect
actual conditions.

> 3)
> Many of the fields in WindowNode (and other new classes added by this patch) are 
> "public", ...

Fixed in patch 16

> 4)
> In WindowNode.java there is a field "level" which is initialized to -1 and then is set
> SelectNode.java.  

Patch 16 adds getters and setters, as well as comments on use of 'level'.

I'm not too satisfied with the variable name 'level' but it was the best 
I could think of...

> 5)
> In ColumnReference.java there are two different kinds of checks to see if the 
> ColumnReference is pointing to a WindowColumnNode.  ...

Yes, the two different approaches are intentional. See updated comment for CR.pointsToWindowFunction().
Fixed to use isWindowFunction() and isVirtualWindowFunction() in patch 16.

> 6)
> See comment #1 above for an alternate (cleaner?) approach...

Patch 16 now uses isWindowFunction() and new isVirtualWindowFunction() for 
this if().

> 7)
> The result set statistics node for WindowResultSet does not currently print out the 
> name of the result set...(I don't think). 

I'll need to look into this.

> 8)
> If I'm understanding correctly, the execution-time logic for WindowResultSet operates

> in a fashion similar to ProjectRestrictResultSet.

True, but more like the 'inverse' of PRRS. The PRRS gets many columns 
from its child resultset, and projects a few of these. The WRS projects 
a few columns into the many destination columns, along with its 
windowfunction result.

>  That is, it iterates through all of its source's rows and, for each one, it applies
> restriction.  If the row fails the restriction, then the iteration moves on to the next
> row, and so on.  This agrees with the following comments, pulled from the 
> getNextRowCore() method of WindowResultSet:
>    /*
>     * Loop until we get a row from the source that qualifies, or there are
>     * no more rows to qualify. For each iteration fetch a row from the
>     * source, and evaluate against the restriction if any.
>     */

What should happen is that the outer selects(S1) PRN calls getNextRow() 
that again calls into the child/subquerys(S2) PRN getNextRowCore(). That 
again calls into the chain of WindowResultSets and PRNs, and eventually 
ends up in a a BulkTableScanResultSet.getNextRowCore() or similar to 
actually fetch *one* row. This single row is then pulled up the 
resultset chain, and the restriction is evaluated at the level it's been 
pushed down to.

In your example S1 evaluates the restriction. It's not 
pushed from S1 into S2 for evaluation as it's a restriction on a window function 
column of S1s subquery/child resultset.
S1 knows that column r is a row number column and that it 
is ascending, so we should stop pulling rows from below once we reach 
the condition.

See reply to #9 too.

> If this is correct, then I'm not entirely sure we'll get the performance boost that one
might perhaps expect from a query like:
>     SELECT * FROM (
>       SELECT row_number() over () as r, t.* FROM T
>     ) AS tmp WHERE r <= 3;
> .... For the above query, the plan shows the following for the top-most 
> ProjectRestrictResultSet:
>   ******* Project-Restrict ResultSet (1):
>   Number of opens = 1
>   Rows seen = 1280
>   Rows filtered = 1277
>   restriction = true

This looks wrong given my above explaination.
It seems we don't take the fact that we know we have an ascending column 
into consideration any more (we used to). I'll have to have another look at this.

I haven't focused on the store layer at all. But based on how many rows 
there are in your table the store layer may load either all or parts of 
your data from disk into its cache. But they should not be pulled up the 
resultset chain to either S2 or S1.

> 9)
> For the query mentioned in comment #8 above, the query plan shows:
>   ProjectRestrictResultSet (sees ALL rows, restricts down to 3)
>     -> ProjectRestrictResultSet (sees ALL rows)
>       -> WindowResultSet (sees ALL rows)
> I intuitively expected the restriction to appear at the level of the WindowResultSet,
> opposed to appearing at the level of the top-most PRN.  The plan as shown above 
> makes it look like the restriction is not being enforced by the Window ResultSet, but
> instead being enforced by the ProjectRestrict. Is that correct?  Or is this just a case

> where the query plan fails to capture what is really happening with the 
> WindowResultSet?  I'm guessing the latter, but am not sure...

This is (more or less) correct.

As explained above, the top PRN must do the restriction, but it should 
never *fetch* all the rows.

I briefly tried pushing the restriction down to the WindowResultSet some time ago, but the
expression generation/execution code does not 
currently want to work with the window function columns.
I did not pursue this further, but agree it should be looked at for a 
future optimization.

However, the code needed to do restriction at the WindowResultSet level 
have been kept in its getNextRowCore(). This is the reason for the loop 
and comment mentioned in reply to #8.

> Add support for ROW_NUMBER() window function
> --------------------------------------------
>                 Key: DERBY-2998
>                 URL: https://issues.apache.org/jira/browse/DERBY-2998
>             Project: Derby
>          Issue Type: Sub-task
>          Components: SQL
>            Reporter: Thomas Nielsen
>            Assignee: Thomas Nielsen
>            Priority: Minor
>         Attachments: d2998-10.diff, d2998-10.stat, d2998-11.diff, d2998-12.diff, d2998-12.stat,
d2998-13.diff, d2998-13.stat, d2998-14.diff, d2998-14.stat, d2998-15.diff, d2998-15.stat,
d2998-16.diff, d2998-16.stat, d2998-4.diff, d2998-4.stat, d2998-5.diff, d2998-5.stat, d2998-6.diff,
d2998-6.stat, d2998-7.diff, d2998-7.stat, d2998-8.diff, d2998-8.stat, d2998-9-derby.log, d2998-9.diff,
d2998-9.stat, d2998-doc-1.diff, d2998-doc-1.stat, d2998-test.diff, d2998-test.stat, d2998-test2.diff,
d2998-test2.stat, d2998-test3.diff, d2998-test3.stat, d2998-test4.diff, d2998-test4.stat,
d2998-test6.diff, d2998-test7.diff
> As part of implementing the overall OLAP Operations features of SQL (DERBY-581), implement
the ROW_NUMBER() window function.
> More information about this feature is available at http://wiki.apache.org/db-derby/OLAPRowNumber

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message