[ http://issues.apache.org/jira/browse/DERBY-147?page=comments#action_12422627 ] Knut Anders Hatlen commented on DERBY-147: ------------------------------------------
Hi Bernd, Thank you for providing the patch. I'm sorry no one has picked it up yet. I had a brief look at it, and I have a couple of comments: * Most of the changes in the patch are white-space changes. This makes it harder for reviewers to understand what parts of the code have actually been changed. If you submit a patch that only contains the actual changes needed to fix the bug, it is much more likely that someone will review it and eventually commit it. * With your changes, the javadoc comment for getOrderByColumn() becomes outdated. It says "ensure that there is only one match" and "@exception StandardException thrown on duplicate". It would be good to fix this as well. * By removing the else-if clause, the code in the else clause is executed instead when you have a duplicate column. This means that duplicate columns are removed from the result list, which is not correct. I believe a correct fix would have to check that index is greater than or equal to (size - orderBySelect) before it calls removeElement() and decOrderBySelect(). The example below illustrates the problem: ij> create table t (col1 int, col2 varchar(10)); 0 rows inserted/updated/deleted ij> insert into t values (1, 'one'), (2, 'two'), (3, 'three'); 3 rows inserted/updated/deleted ij> select col1, col2, col1 from t order by col1; COL1 |COL2 ---------------------- 1 |one 2 |two 3 |three 3 rows selected COL1 should have occurred twice in the result. * It would be really great if the patch had a regression test. It doesn't have to be more sophisticated than the query above. It's as simple as adding the query to java/testing/org/apache/derbyTesting/functionTests/tests/lang/orderby.sql and updating the master java/testing/org/apache/derbyTesting/functionTests/master/orderby.out with the expected results. * The error message that was removed, is not used anywhere else in the code. Therefore, LANG_DUPLICATE_COLUMN_FOR_ORDER_BY could be removed from SQLState.java, and the corresponding error message could be removed from java/engine/org/apache/derby/loc/messages_en.properties. Thanks, Knut Anders > ERROR 42X79 not consistant ? - same column name specified twice > --------------------------------------------------------------- > > Key: DERBY-147 > URL: http://issues.apache.org/jira/browse/DERBY-147 > Project: Derby > Issue Type: Bug > Components: SQL > Reporter: Bernd Ruehlicke > Attachments: derby-147-10.0.2.1.diff, derby-147.diff > > > This happens from JDBC or ij. Here the output form ij> > ij version 10.0 > CONNECTION0* - jdbc:derby:phsDB > * = current connection > ij> select a1.XXX_foreign, a1.native, a1.kind, a1.XXX_foreign FROM > slg_name_lookup a1 ORDER BY a1.XXX_foreign; > ERROR 42X79: Column name 'XXX_FOREIGN' appears more than once in the result > of the query expression. > But when removing the ORDER BY and keeping the 2 same column names it works > ij> select a1.XXX_foreign, a1.native, a1.kind, a1.XXX_foreign FROM > slg_name_lookup a1; > XXX_FOREIGN > |NATIVE > |KIND |XXX_FOREIGN > > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- > > 0 rows selected > ij> > So - it seams to be OK to specify the same column twice - as long as you do > not add the ORDER BY clause. > I woul dof course like that the system allows this - but at leats it should > be consistant and either allow both or none of the two queries above. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
