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-4371) Non-selected columns for SELECT DISTINCT allowed in ORDER BY clause if ordered by expression
Date Mon, 23 Aug 2010 15:18:17 GMT

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

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

Thanks Nirmal, the new patch was easier to read. And thanks for adding all the test cases.

I believe the code is correct, but it's still somewhat tricky to understand, especially because
of the nesting levels and the way it breaks out of the loops. Here are some suggestions that
I think would make the code easier to follow:

1) Reduce the nesting level in the else branch in bindOrderByColumn(). Instead of the current
code with two levels

        if (addedColumnOffset >= 0 && ....) {
            if (!expressionMatch(target)) {

merge them into one if statement:

    if (addedColumnOffset >= 0 && ....
        && !expressionMatch(target)) {

2. Instead of breaking out of the loop in bindOrderByColumn() with the "break" keyword when
a non-matching column is found, just throw the exception immediately. Then it's easier to
see what the effect of a mismatch is.

3. Given that the "throw" statement is moved inside the loop as part of comment (2), there's
no need to check explicitly whether the list of collected nodes was empty, so "if(collectNodesVisitor.getList().isEmpty())"
can be removed.

4. The results of some sub-expressions could be put in to local variables to avoid having
to repeat them. For example, target.getResultColumns() in expressionMatch(), and target.getResultColumns()
and target.getResultColumns().getResultColumn(i).getExpression() in columnMatchFound().

5. The loop in columnMatchFound() could be simplified by returning true immediately when a
match is found, instead of setting a flag and then checking the flag further down in the loop
body.

> Non-selected columns for SELECT DISTINCT allowed in ORDER BY clause if ordered by expression
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-4371
>                 URL: https://issues.apache.org/jira/browse/DERBY-4371
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>    Affects Versions: 10.5.1.1
>            Reporter: Bernt M. Johnsen
>            Assignee: C.S. Nirmal J. Fernando
>            Priority: Critical
>         Attachments: DERBY-4371-2.diff, DERBY-4371-3.diff, DERBY-4371-4.diff, derby-4371-5.diff,
derby-4371-6.diff, derby-4371-tests.diff, DERBY-4371.diff
>
>
> How to repeat:
> ij> create table t (i integer, j integer);;
> 0 rows inserted/updated/deleted
> ij> insert into t values (1,2),(1,3);
> 2 rows inserted/updated/deleted
> ij> select distinct i from t order by j;
> ERROR 42879: The ORDER BY clause may not contain column 'J', since the query specifies
DISTINCT and that column does not appear in the query result.
> ij> select distinct i from t order by j*2;
> I          
> -----------
> 1          
> 1          
> 2 rows selected

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