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-2998) Add support for ROW_NUMBER() window function
Date Tue, 26 Feb 2008 18:02:51 GMT

    [ https://issues.apache.org/jira/browse/DERBY-2998?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12572580#action_12572580
] 

A B commented on DERBY-2998:
----------------------------

> Is this something that has been fixed with patch 17? Or is it still work in progress?

Okay, I ran the same queries with patch 17 and can see for myself that this particular change
has not yet been made.

On patch 17 itself I have a few more comments:

1) The patch adds the following to ResultColumnList:

   /*
    * To avoid messing up the correlation flags we should determine if any
    * of this RCLs columns are either:
    * - A windowfunction column
    * - A virtual column node pointing to window function column
    */

I think it would be nice to expand a bit on *why* this code is necessary. The comments indicate
that without the code we could "mess up the correlation flags", but it doesn't say how. At
a minimum it would be good to have a pointer to the correlation code that is affected by this.

2) Code in WindowNode.generate(...) ends with a check and a comment about positioned updates
and deletes. Are there test cases to ensure all is okay with such operations?

3) On more a general level, and especially with respect to comment # 5 in my Feb 25th post,
it seems like the changes for this issue rely heavily on the expected appearance of a WindowFunctionColumnNode
in the query tree. Namely, the fact that

  ResultColumn -> WindowFunctionColumn

is expected in some places and

  ResultColumn -> VirtualColumnNode -> WindowFunctionColumn

is expected in others makes me nervous. My experience with optimization/compilation is that
VirtualColumnNodes, ResultColumns, and ColumnReferences are often stacked on top of each other
in rather gratuitous fashion. Code which accesses these types of nodes is usually (or at least,
often) written with the expectation that it could be dealing with VCN, ResultColumn, and/or
ColRef chains of arbitrary length, and so must walk the chain when retrieving certain information.

But the changes for ROW_NUMBER() take a different approach, one in which the assumption is
that we will always have either 1) a ResultColumn whose child is a WindowFunctionColumn, or
2) a ResultColumn whose child is a VirtualColumnNode whose child is a WindowFunctionColumn.
But how confident are we that this will _always_ be the case?

With this thought in my head I can't help but feel that queries which involve ROW_NUMBER()
and which are larger or more complex are at a high risk for column "mis-information", which
could cause issues. I applied patch 17 and tried writing some more complicated queries that
use ROW_NUMBER(), and it wasn't long before I hit the ASSERT failure that we've seen before.
Esp:

-- OK
select row_number() over () from t1 union all select row_number() over () from t1;
select 2 * r from (select row_number() over () from t1) x(r);

-- Fail.
select count(*) from (select row_number() over() from t1) x;
select count(*) from (select row_number() over () as r from t1) as t(r) where r <=3;

-- OK
select c3, c1, c2 from
  (select i, j, row_number() over() as r from t1) x1 (c1, c2, r1),
  (select row_number() over() as r, j, i from t1) x2 (r2, c3, c4);

-- OK
select c3, c1, c2 from
  (select i, j, row_number() over() as r from t1) x1 (c1, c2, r1),
  (select row_number() over() as r, j, i from t1) x2 (r2, c3, c4),
  t1;

-- OK
select c3, c1, c2 from
  (select i, j, row_number() over() as r from t1) x1 (c1, c2, r1),
  (select row_number() over() as r, j, i from t1) x2 (r2, c3, c4),
  t1
 where x1.r1 = 2 * x2.r2;

-- Fails
select c3, c1, c2 from
  (select i, j, row_number() over() as r from t1) x1 (c1, c2, r1),
  (select row_number() over() as r, j, i from t1) x2 (r2, c3, c4),
  t1
 where x2.c4 = t1.i;

-- Fails
select c3, c1, c2 from
  (select i, j, row_number() over() as r from t1) x1 (c1, c2, r1),
  (select row_number() over() as r, j, i from t1) x2 (r2, c3, c4),
  t1
 where x1.r1 = 2 * x2.r2 and x2.c4 = t1.i;

Perhaps these failures have nothing to do with my "fear" outlined above--in which case I apologize
of the misdirection. But in any event, there are still some failures that I see with patch
17. I'm not sure to what extent these need to be resolved before code can be committed, as
they (appear to) only occur when ROW_NUMBER() is in use and thus shouldn't affect existing
apps. But I'm not the one to say for sure one way or the other...

> 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-17.diff, d2998-17.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.


Mime
View raw message