Daniel John Debrunner wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Comments on the patch 'Derby-13.diff' attached to the Jira bug Derby-13 > > I think we are making progress, but some comments: > > - - How do you (or someone else) know the bug is fixed if there are no > test cases added for it?
I can give a shot at writing tests. > > - - I would expect a more complete change to use TableName, rather than > String for exposed name. That is, an exposed name is always handled as a > TableName and not a string. Your approach is to keep the exposed name as > a String and convert it to a TableName as required. I think it will be a > lot cleaner if it is always a TableName and never a String. Ie. > getExposedName() returns a TableName. In java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java the exposedTableName object is built using the makeTableName method. I feel changing the signature of the getExposedName() method would be a much larger operation and users might not like the change as well as the current method just returns a String and the new method would return a TableName object. > I think your code then leads to a problem, where the schema name for an > exposed name is incorrect, as you take the schema name of the table, not > the current schema. Though I could be wrong on this. Namely in this > situation, what is the actual value for the exposed (correlation) name? > > set current schema A; > > select * from B.T1 AS C > > > What is the correlation name > > C (no associated schema?) > A.C > B.C > > ?? The correlation name would be "C" ( no associated schema ). This is because for the query "select * from B.T1 AS C", "*" is not associated with a table name, that is in the code allTableName will be null and hence no schema is associated with C. (although the default schema is A) > > I briefly looked through the SQL spec a while ago on this issue and > couldn't find the answer. I agree the SQL spec does not talk about this, hence the above solution. > > Dan. > > Can all of you kindly vote for the patch ? thanks Shreyas
