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