You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by Shreyas Kaushik <Sh...@Sun.COM> on 2005/02/01 14:46:06 UTC
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Any updates on this??
thanks
Shreyas
Shreyas Kaushik wrote:
> I am attaching the latest diffs for Derby-18.
>
> This tries to address Dan's comments in his previous mails.
> Let me know the whether the extra checks that I have added, in
> comparision to the diffs I sent out earlier
> are correct/sufficient.
>
> thanks
> Shreyas
>
> Shreyas Kaushik wrote:
>
>> My answers inline.
>>
>> Shreyas
>>
>> Daniel John Debrunner wrote:
>>
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> Shreyas Kaushik wrote:
>>>
>>>
>>>
>>>> Dan,
>>>>
>>>> I did the patch for this.
>>>>
>>>> From your comments I looked at the getSchemaDescriptor() in
>>>> QueryTreeNode. The schema name was getting
>>>> stored there but not stored in the TableName in the corresponding
>>>> node.
>>>>
>>>> Since *null* schema implies current schema I just called bind() in
>>>> TableName.java whereever the schema was null.
>>>>
>>>
>>>
>>>
>>> I'm not sure this is correct, because if you remember you patch to
>>> Derby-13, a TableName with a null schema can represent a correlation
>>> name (which don't have schemas). So the fact a TableName's schema name
>>> is null does not always mean that its schema name maps to the current
>>> schema. Thus only when a TableName represents a implicitly schema
>>> qualifed name (table, view, procedure, function, trigger, index,
>>> constraint name) should its lack of schema name be converted to the
>>> current schema. Possibly the only case that doesn't have an implicit
>>> schema name is the correlation name.
>>>
>>>
>> Yes you are right, I agree .
>>
>>> Thus the patch needs to be careful of this overloaded use of TableName.
>>> Does your patch handle this correctly?
>>
>>
>> While binding the TableName object with schema name see whether
>> correlation nae is null or not, along these lines.
>>
>>> Without looking at the code in
>>> detail I'm not sure how to handle this correctly, if you haven't made
>>> any progress I can try give and help out tomorrow.
>>>
>>>
>> Your help and comments would be helpful for me to learn going
>> forward. Thanks :-)
>>
>>> Dan.
>>>
>>> -----BEGIN PGP SIGNATURE-----
>>> Version: GnuPG v1.2.5 (MingW32)
>>> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>>>
>>> iD8DBQFB+c6jIv0S4qsbfuQRAvmcAJ9Nu4H0HzhGxg+S8nOJqBxi1gbbAQCgtWjO
>>> dLSVBATTSeJjkHrin45ygk4=
>>> =3KaU
>>> -----END PGP SIGNATURE-----
>>>
>>>
>>>
>------------------------------------------------------------------------
>
>Index: java/engine/org/apache/derby/impl/sql/compile/FromList.java
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/FromList.java (revision 148848)
>+++ java/engine/org/apache/derby/impl/sql/compile/FromList.java (working copy)
>@@ -138,6 +138,7 @@
> */
> TableName leftTable = null;
> TableName rightTable = null;
>+
> if (! (fromTable instanceof TableOperatorNode))
> {
> /* Check for duplicate table name in FROM list */
>@@ -146,12 +147,18 @@
> {
> leftTable = fromTable.getTableName();
>
>+ if(leftTable.getSchemaName() == null && fromTable.correlationName == null)
>+ leftTable.bind(this.getDataDictionary());
>+
> if(((FromTable) elementAt(index)) instanceof TableOperatorNode) {
> continue;
> }
>
>- else {
>+ else {
> rightTable = ((FromTable) elementAt(index)).getTableName();
>+
>+ if(rightTable.getSchemaName() == null && ((FromTable) elementAt(index)).correlationName == null)
>+ rightTable.bind(this.getDataDictionary());
> }
> if(leftTable.equals(rightTable))
> {
>@@ -291,6 +298,7 @@
> {
> fromTable = (FromTable) elementAt(index);
> setElementAt(fromTable.bindNonVTITables(dataDictionary, fromListParam), index);
>+
> }
> for (int index = 0; index < size; index++)
> {
>@@ -499,9 +507,9 @@
> int size = size();
> for (int index = 0; index < size; index++)
> {
>- fromTable = (FromTable) elementAt(index);
>+ fromTable = (FromTable) elementAt(index);
>
>- /* We can stop if we've found a matching column or table name
>+ /* We can stop if we've found a matching column or table name
> * at the previous nesting level.
> */
> currentLevel = fromTable.getLevel();
>Index: java/engine/org/apache/derby/impl/sql/compile/FromVTI.java
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/FromVTI.java (revision 148848)
>+++ java/engine/org/apache/derby/impl/sql/compile/FromVTI.java (working copy)
>@@ -988,6 +988,13 @@
>
> columnsTableName = columnReference.getTableNameNode();
>
>+ if(columnsTableName != null)
>+ if(columnsTableName.getSchemaName() == null && correlationName == null)
>+ columnsTableName.bind(this.getDataDictionary());
>+
>+ if(exposedName != null)
>+ if(exposedName.getSchemaName() == null && correlationName == null)
>+ exposedName.bind(this.getDataDictionary());
> /*
> ** If the column did not specify a name, or the specified name
> ** matches the table we're looking at, see whether the column
>Index: java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java (revision 148848)
>+++ java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java (working copy)
>@@ -2125,6 +2125,7 @@
> {
> TableDescriptor tableDescriptor = bindTableDescriptor();
>
>+
> /* if (tableDescriptor.getTableType() == TableDescriptor.VTI_TYPE) {
> ResultSetNode vtiNode = getNodeFactory().mapTableAsVTI(getContextManager(), tableDescriptor);
> return vtiNode.bindNonVTITables(dataDictionary, fromListParam);
>@@ -2445,6 +2446,10 @@
>
> columnsTableName = columnReference.getTableNameNode();
>
>+ if(columnsTableName != null) {
>+ if(columnsTableName.getSchemaName() == null && correlationName == null)
>+ columnsTableName.bind(this.getDataDictionary());
>+ }
> /*
> ** If there is a correlation name, use that instead of the
> ** table name.
>@@ -2458,6 +2463,8 @@
> exposedTableName = tableName;
> }
>
>+ if(exposedTableName.getSchemaName() == null && correlationName == null)
>+ exposedTableName.bind(this.getDataDictionary());
> /*
> ** If the column did not specify a name, or the specified name
> ** matches the table we're looking at, see whether the column
>@@ -2465,6 +2472,7 @@
> */
> if (columnsTableName == null || columnsTableName.equals(exposedTableName))
> {
>+
> resultColumn = resultColumns.getResultColumn(columnReference.getColumnName());
> /* Did we find a match? */
> if (resultColumn != null)
>@@ -2480,8 +2488,7 @@
> tableDescriptor.setReferencedColumnMap(referencedColumnMap);
> }
> }
>- }
>-
>+ }
> return resultColumn;
> }
>
>Index: java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java (revision 148848)
>+++ java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java (working copy)
>@@ -305,6 +305,10 @@
>
> columnsTableName = columnReference.getTableNameNode();
>
>+ if(columnsTableName != null)
>+ if(columnsTableName.getSchemaName() == null && correlationName == null)
>+ columnsTableName.bind(this.getDataDictionary());
>+
> if (SanityManager.DEBUG)
> {
> SanityManager.ASSERT(preStmt!=null, "must have prepared statement");
>@@ -324,6 +328,11 @@
> SanityManager.ASSERT(baseTableName!=null,"no name on target table");
> }
>
>+ if(baseTableName != null)
>+ if(baseTableName.getSchemaName() == null && correlationName == null)
>+ baseTableName.bind(this.getDataDictionary());
>+
>+
> /*
> * If the column did not specify a name, or the specified name
> * matches the table we're looking at, see whether the column
>
>
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Satheesh Bandaram wrote:
> I tried the previously failing create view statement on DB2 that I have
> access to. DB2 matches the new behaviour.... View creation passes with
> the schema name qualification and fails without. Derby has had issues in
> the past, getting confused between current schema and the schema of the
> object during compilation.
>
> But I am not sure if this behavior is desirable. The new behavior has
> the schema name in the front for error messages that the user did not
> specify. Can cause confusion to users.
>
> ij> select * from test.s ss (c1, c2, c3, c4) where exists (select *ss.i
> *from test.tt);
> -ERROR 42X04: Column *'SS.I' *is not in any table in the FROM list or it
> appears within a join specification and is outside the scope of the join
> specification or it appears in a HAVING clause and is not in the GROUP
> BY list. If this is a CREATE or ALTER TABLE statement then 'SS.I' is
> not a column in the target table.
> +ERROR 42X04: Column *'APP.SS.I'* is not in any table in the FROM list
> or it appears within a join specification and is outside the scope of
> the join specification or it appears in a HAVING clause and is not in
> the GROUP BY list. If this is a CREATE or ALTER TABLE statement then
> 'APP.SS.I' is not a column in the target table.
Thanks Sathessh for confirming that the dml162 change is correct.
I had the same concern about the error message change, I'm trying a
quick change where TableName remembers if it was initialized with a
schema name, and then uses that information in its toString() method,
so that the above example should only return SS.I.
Dan.
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Shreyas Kaushik wrote:
>> Dan Debrunner wrote:
>> The dml162 test from the
>> nist suite fails to create a view that previously was created ok. This
>> dml162 issue would need to be resolved before the patch could be
>> accepted.
>>
>>
> With the changes the fully qualified column name should be given or it
> would take the default schema and the
> default schema does not have what the test is looking for. Since
> everything is prefixed with "HU." so should the
> column in question. With that change it works fine. Sending a patch for
> this also.
> Index: dml162.sql
> ===================================================================
> --- dml162.sql (revision 153026)
> +++ dml162.sql (working copy)
> @@ -21,7 +21,7 @@
> CREATE VIEW BLIVET (CITY, PNUM, EMPNUM, EMPNAME, GRADE,
> HOURS, PNAME, PTYPE, BUDGET) AS
> --0 HU.STAFF NATURAL JOIN HU.WORKS NATURAL JOIN HU.PROJ;
> - SELECT PROJ.CITY, HU.PROJ.PNUM, HU.STAFF.EMPNUM, EMPNAME, GRADE, HOURS, PNAME, PTYPE, BUDGET
> + SELECT HU.PROJ.CITY, HU.PROJ.PNUM, HU.STAFF.EMPNUM, EMPNAME, GRADE, HOURS, PNAME, PTYPE, BUDGET
> FROM HU.STAFF JOIN HU.WORKS ON (HU.STAFF.EMPNUM=HU.WORKS.EMPNUM) JOIN HU.PROJ ON (HU.PROJ.PNUM=HU.WORKS.PNUM AND HU.PROJ.CITY=HU.STAFF.CITY)
> ;
> -- PASS:0863 If view created successfully?
Anyone else have any comments on this patch, was the view in dml162
being created incorrectly before? The view has been modified from the
original NIST code, so maybe it was always a hidden bug in Cloudscape.
Dan.
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Shreyas Kaushik wrote:
>>> Attached is the new patch and answesr are inline.
Patch applied as revision 157591
Dan.
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Shreyas Kaushik <Sh...@Sun.COM>.
Dan,
Please see below.
~ Shreyas
Daniel John Debrunner wrote:
>><>Shreyas Kaushik wrote:
>>
>>Attached is the new patch and answesr are inline.
>>
>>
>>
>>
>>
>>
>Daniel John Debrunner wrote:
>
>
>
>>I got the patch to apply and am running tests on it.
>>
>>
>>
>>
>>
>9 (possibly with one unrelated failure) tests fail or have changed
>output with this patch. Most of the differences in the output seem to be
>a change in the reporting of the failing column to include the implicit
>schema name, e.g. T1.C1 is replaced by APP.T1.C1 in an error message.
>However, there are a few troublesome diffs. The dml162 test from the
>nist suite fails to create a view that previously was created ok. This
>dml162 issue would need to be resolved before the patch could be accepted.
>
>
With the changes the fully qualified column name should be given or it
would take the default schema and the
default schema does not have what the test is looking for. Since
everything is prefixed with "HU." so should the
column in question. With that change it works fine. Sending a patch for
this also.
>In addition there are no additional, new, test cases that show the
>change in behaviour to demostrate that Derby-18 is indeed fixed. Test
>cases can be added to existing tests.
>
>
Sending a patch having new tests ( 2 tests ) that check the fix.
>Again, like the identifier change, if a patch from a contibutor causes
>the tests to "fail" due to modified but correct output, who should be
>fixing up the test masters? I'd assume the contributor.
>
>
Sending the patch for the master files as well.
>Dan.
>
>
>
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Daniel John Debrunner <dj...@debrunners.com>.
> <>Shreyas Kaushik wrote:
>
> Attached is the new patch and answesr are inline.
>
>
>
>
Daniel John Debrunner wrote:
>I got the patch to apply and am running tests on it.
>
>
>
9 (possibly with one unrelated failure) tests fail or have changed
output with this patch. Most of the differences in the output seem to be
a change in the reporting of the failing column to include the implicit
schema name, e.g. T1.C1 is replaced by APP.T1.C1 in an error message.
However, there are a few troublesome diffs. The dml162 test from the
nist suite fails to create a view that previously was created ok. This
dml162 issue would need to be resolved before the patch could be accepted.
In addition there are no additional, new, test cases that show the
change in behaviour to demostrate that Derby-18 is indeed fixed. Test
cases can be added to existing tests.
Again, like the identifier change, if a patch from a contibutor causes
the tests to "fail" due to modified but correct output, who should be
fixing up the test masters? I'd assume the contributor.
Dan.
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Shreyas Kaushik wrote:
> Dan,
>
> Attached is the new patch and answesr are inline.
I got the patch to apply and am running tests on it.
Dan.
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Shreyas Kaushik <Sh...@Sun.COM>.
Any updates here?
~ Shreyas
Shreyas Kaushik wrote:
> Dan,
>
> Attached is the new patch and answesr are inline.
>
> ~ Shreyas.
>
> Daniel John Debrunner wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Shreyas Kaushik wrote:
>>
>>
>>> Any updates on this??
>>>
>>> thanks
>>> Shreyas
>>>
>>> Shreyas Kaushik wrote:
>>>
>>>
>>>
>>>> I am attaching the latest diffs for Derby-18.
>>>>
>>>> This tries to address Dan's comments in his previous mails.
>>>> Let me know the whether the extra checks that I have added, in
>>>> comparision to the diffs I sent out earlier
>>>> are correct/sufficient.
>>>>
>>>
>>
>> I've been busy doing other stuff, from a quick look at this I'm was more
>> thinking that the schema name in TableName would be filled in at a
>> single point, rather than on each use of it. This latter approach tends
>> to increase code size and be more buggy as there is potential for some
>> uses to be missed. I would expect that this could be encapsulated in
>> FromTable or FromBaseTable, maybe additional checks in the getTableName
>> method?
>>
>>
> Ok I guess you are right, a good appraoch at doing things more
> concisely. I have made the changes
> in FromBaseTableName and CurrentOfNode.
>
>> Dan.
>>
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.2.5 (MingW32)
>> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>>
>> iD8DBQFCAB/dIv0S4qsbfuQRAjRlAJ9ujj9gLG7mnuDmi2UMbVIHV/aDpACePKas
>> s8t0EM82u63D1UST+24LQSk=
>> =+rpI
>> -----END PGP SIGNATURE-----
>>
>>
>>
>------------------------------------------------------------------------
>
>Index: java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java (revision 149478)
>+++ java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java (working copy)
>@@ -2125,6 +2125,7 @@
> {
> TableDescriptor tableDescriptor = bindTableDescriptor();
>
>+
> /* if (tableDescriptor.getTableType() == TableDescriptor.VTI_TYPE) {
> ResultSetNode vtiNode = getNodeFactory().mapTableAsVTI(getContextManager(), tableDescriptor);
> return vtiNode.bindNonVTITables(dataDictionary, fromListParam);
>@@ -2445,6 +2446,10 @@
>
> columnsTableName = columnReference.getTableNameNode();
>
>+ if(columnsTableName != null) {
>+ if(columnsTableName.getSchemaName() == null && correlationName == null)
>+ columnsTableName.bind(this.getDataDictionary());
>+ }
> /*
> ** If there is a correlation name, use that instead of the
> ** table name.
>@@ -2458,6 +2463,8 @@
> exposedTableName = tableName;
> }
>
>+ if(exposedTableName.getSchemaName() == null && correlationName == null)
>+ exposedTableName.bind(this.getDataDictionary());
> /*
> ** If the column did not specify a name, or the specified name
> ** matches the table we're looking at, see whether the column
>@@ -2465,6 +2472,7 @@
> */
> if (columnsTableName == null || columnsTableName.equals(exposedTableName))
> {
>+
> resultColumn = resultColumns.getResultColumn(columnReference.getColumnName());
> /* Did we find a match? */
> if (resultColumn != null)
>@@ -2480,8 +2488,7 @@
> tableDescriptor.setReferencedColumnMap(referencedColumnMap);
> }
> }
>- }
>-
>+ }
> return resultColumn;
> }
>
>@@ -3572,6 +3579,13 @@
> TableName tn;
>
> tn = super.getTableName();
>+
>+ if(tn != null) {
>+ if(tn.getSchemaName() == null &&
>+ correlationName == null)
>+ tn.bind(this.getDataDictionary());
>+ }
>+
> return (tn != null ? tn : tableName);
> }
>
>Index: java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java (revision 149478)
>+++ java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java (working copy)
>@@ -305,6 +305,10 @@
>
> columnsTableName = columnReference.getTableNameNode();
>
>+ if(columnsTableName != null)
>+ if(columnsTableName.getSchemaName() == null && correlationName == null)
>+ columnsTableName.bind(this.getDataDictionary());
>+
> if (SanityManager.DEBUG)
> {
> SanityManager.ASSERT(preStmt!=null, "must have prepared statement");
>@@ -324,7 +328,11 @@
> SanityManager.ASSERT(baseTableName!=null,"no name on target table");
> }
>
>- /*
>+ if(baseTableName != null)
>+ if(baseTableName.getSchemaName() == null && correlationName == null)
>+ baseTableName.bind(this.getDataDictionary());
>+
>+ /*
> * If the column did not specify a name, or the specified name
> * matches the table we're looking at, see whether the column
> * is in this table, and also whether it is in the for update list.
>
>
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Shreyas Kaushik <Sh...@Sun.COM>.
Dan,
Attached is the new patch and answesr are inline.
~ Shreyas.
Daniel John Debrunner wrote:
>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>Shreyas Kaushik wrote:
>
>
>>Any updates on this??
>>
>>thanks
>>Shreyas
>>
>>Shreyas Kaushik wrote:
>>
>>
>>
>>>I am attaching the latest diffs for Derby-18.
>>>
>>>This tries to address Dan's comments in his previous mails.
>>>Let me know the whether the extra checks that I have added, in
>>>comparision to the diffs I sent out earlier
>>>are correct/sufficient.
>>>
>>>
>
>I've been busy doing other stuff, from a quick look at this I'm was more
>thinking that the schema name in TableName would be filled in at a
>single point, rather than on each use of it. This latter approach tends
>to increase code size and be more buggy as there is potential for some
>uses to be missed. I would expect that this could be encapsulated in
>FromTable or FromBaseTable, maybe additional checks in the getTableName
>method?
>
>
Ok I guess you are right, a good appraoch at doing things more
concisely. I have made the changes
in FromBaseTableName and CurrentOfNode.
>Dan.
>
>-----BEGIN PGP SIGNATURE-----
>Version: GnuPG v1.2.5 (MingW32)
>Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
>iD8DBQFCAB/dIv0S4qsbfuQRAjRlAJ9ujj9gLG7mnuDmi2UMbVIHV/aDpACePKas
>s8t0EM82u63D1UST+24LQSk=
>=+rpI
>-----END PGP SIGNATURE-----
>
>
>
Re: [jira] Commented: (DERBY-18) Exposed name matching has bugs when
the column name is qualified with a schema name.
Posted by Daniel John Debrunner <dj...@debrunners.com>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Shreyas Kaushik wrote:
> Any updates on this??
>
> thanks
> Shreyas
>
> Shreyas Kaushik wrote:
>
>> I am attaching the latest diffs for Derby-18.
>>
>> This tries to address Dan's comments in his previous mails.
>> Let me know the whether the extra checks that I have added, in
>> comparision to the diffs I sent out earlier
>> are correct/sufficient.
I've been busy doing other stuff, from a quick look at this I'm was more
thinking that the schema name in TableName would be filled in at a
single point, rather than on each use of it. This latter approach tends
to increase code size and be more buggy as there is potential for some
uses to be missed. I would expect that this could be encapsulated in
FromTable or FromBaseTable, maybe additional checks in the getTableName
method?
Dan.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFCAB/dIv0S4qsbfuQRAjRlAJ9ujj9gLG7mnuDmi2UMbVIHV/aDpACePKas
s8t0EM82u63D1UST+24LQSk=
=+rpI
-----END PGP SIGNATURE-----