-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Shreyas Kaushik wrote:
> Yes it does. > > Daniel John Debrunner wrote: > >> >>Does the fix correctly implement duplicate name checking as detailed in >>this mail? >> >>http://mail-archives.apache.org/eyebrowse/[EMAIL PROTECTED]&msgNo=1569 Hmmm, I don't see how the change in FromList (extracted below) will correctly handle select ... FROM A.T1 as C, B.T2 as C It seems that the leftTable variable will be set to <A.C> and rightTable set to <B.C> which will not compare as equal. Am I missing something? If the patch had tests I could see if this case was handled. The code in FromList is not that clear to me, could be helped by comments, and it concerns me that a TableName object is created out of a schema name and a correlation name when such a construct has no valid meaning. I see that you are hampered by the fact that there is no way to tell if a correlation name has been used. In msgNo 1569 I laid out that it would be best if there were two getExposed methods, but now I think a single String getExposedName() will do, as long as it returns null if no correlation name was used. That might have a ripple effect though the code, but the best way to fix a bug is sometimes a minor rework, rather than just band-aids on existing bad code. I think the fact that exposed name handling has three bugs shows a re-work is needed. In any re-work I would start out with explicitly stating how exposed names are handled for a table in the from list and for a column reference, and then how the two uses relate and compare. Other comments on the patch are: - - for the code extract below as a minor nit I would avoid using terms like 'leftTable' or 'rightTable' unless left and right actual have meaning, which they don't here. One table in a list is being compared against the others in the list. - - The other set of changes related to getAllResultColumns() again have the possible issue of creating a TableName out of a schema name and correlation name. I assume these are to fix derby-12, again comments would be useful. Also any comments you have on if this fixes Derby-18 and Derby-12 would be helpful. Dan. - - if (fromTable.getExposedName().equals - - (((FromTable) elementAt(index)).getExposedName()) ) + leftTable = makeTableName( fromTable.getTableName().getSchemaName(), + fromTable.getExposedName()); + + if(((FromTable) elementAt(index)).getTableName() == null) { + rightTable = makeTableName(null, ((FromTable) elementAt(index)).getExposedName()); + } + + else { + rightTable = makeTableName(((FromTable) elementAt(index)).getTableName().getSchemaName(), + ((FromTable) elementAt(index)).getExposedName()); + } + if(leftTable.equals(rightTable)) { throw StandardException.newException(SQLState.LANG_FROM_LIST_DUPLICATE_TABLE_NAME, fromTable.getExposedName()); } -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.5 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFB44TBIv0S4qsbfuQRAjgCAKC7xhogSSJgQS3YeGVLxiM3ePdBmgCgqGd2 jRQZ5MXv5Z6nti9/r5iwQDo= =FFKt -----END PGP SIGNATURE-----
