db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DERBY-6378) OFFSET/FETCH NEXT ignored when query is enclosed in parentheses
Date Mon, 28 Oct 2013 10:37:30 GMT

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

Knut Anders Hatlen commented on DERBY-6378:
-------------------------------------------

Thanks, Dag. The approach in the #3 patch looks good to me.

Some comments:

- It would be good to add a sentence to the javadoc comments of pushOrderBy() and pushOffsetFetchFirst()
about the requirement to call pushQueryExpressionSuffix() first.
- The "test is necessary" comment in CursorNode is a bit cryptic.
- RSN.pushQueryExpressionSuffix() could have a javadoc comment saying what it does (currently
it only says who should override it).
- RSN.pushQueryExpressionSuffix() could be made package-private (its siblings pushOrderByList()
and pushOffsetFetchFirst() already are).
- QueryExpressionClauses does not reference its parent instance, so it may be declared static.
Since the class is package-private, its methods are effectively non-public, so it might be
clearer not to declare them with the "public" modifier.
- QueryExpressionClauses.hasOffsetFetchFirst() could access the offset and fetchFirst fields
directly without going through the getter methods and thereby avoid creating temporary array
objects.
- Code such as {{qec.getFetchFirsts()\[i\] = (ValueNode)fetchFirst.accept\(v\);}} looks a
bit suspicious, since getFetchFirsts() returns a fresh array, so the assignment will only
be performed on an array that is immediately eligible for gc. (Similar code exists for getOrderByLists()
and getOffsets() too.) Was it supposed to update the QEC's ArrayList instead?
- All calls to getFetchFirsts() and friends seem to look at only one element of the returned
array and throw the rest of the array away. We might change these getters to take the level
as an argument and return a single node instead. Then we don't need the temporary arrays.


> OFFSET/FETCH NEXT ignored when query is enclosed in parentheses
> ---------------------------------------------------------------
>
>                 Key: DERBY-6378
>                 URL: https://issues.apache.org/jira/browse/DERBY-6378
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>    Affects Versions: 10.10.1.1
>            Reporter: Knut Anders Hatlen
>            Assignee: Dag H. Wanvik
>         Attachments: derby-6378-1.diff, derby-6378-2.diff, derby-6378-2.status, derby-6378-3.diff,
derby-6378-3.status
>
>
> ij version 10.10
> ij> connect 'jdbc:derby:memory:db;create=true';
> ij> create table t(x int);
> 0 rows inserted/updated/deleted
> ij> insert into t values 1,2,3;
> 3 rows inserted/updated/deleted
> ij> select * from t offset 1 row fetch first 1 row only;
> X          
> -----------
> 2          
> 1 row selected
> ij> (select * from t offset 1 row fetch first 1 row only);
> X          
> -----------
> 1          
> 2          
> 3          
> 3 rows selected
> I would have expected that both of the queries had returned a single row.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Mime
View raw message