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 (JIRA)" <de...@db.apache.org> on 2005/01/24 13:32:17 UTC

[jira] Commented: (DERBY-18) Exposed name matching has bugs when the column name is qualified with a schema name.

     [ http://issues.apache.org/jira/browse/DERBY-18?page=comments#action_58008 ]
     
Shreyas Kaushik commented on DERBY-18:
--------------------------------------

set current schema app;

create table t1(int c1 varchar(10));

create schema s1;
create table s1.t1(id1 int, d2 varchar(10));

select * from t1, app.t1;  ---> This fails, should succeed.

Internally table names are handled correctly but the above problem occurs because of the equals() method in Tablename.java. 

Here the follwoing piece of code 

----------------------------------------------------------------------------------
else if ((schemaName == null) ||
				 (otherTableName.getSchemaName() == null))
		{
			return tableName.equals(otherTableName.getTableName());
	     }
----------------------------------------------------------------------------------

causes the above error to occur. In this if either of the schema name is null, only the table names are compared and hence the error for the above statement.

Should the equals method implementation be changed to take of this?
  or
Should we set the tables that have null schemas to the current schema or something along these lines?





> Exposed name matching has bugs when the column name is qualified with a schema name.
> ------------------------------------------------------------------------------------
>
>          Key: DERBY-18
>          URL: http://issues.apache.org/jira/browse/DERBY-18
>      Project: Derby
>         Type: Bug
>   Components: SQL
>     Versions: 10.0.2.0
>     Reporter: Tulika Agrawal
>     Priority: Minor

>
> Reporting for Kathey Marsden.
> create table t1(c1 int);  -- goes into app
> select sys.t1.c1 from t1; -- should fail
> select sys.b.c1 from t1 b; -- should fail
> select * from t1, app.t1;  -- fails, should succeed
> select t1.c1 from t1, app.t1; -- fails, should succeed
> - According to SQL92, the <table 
> name> in a <table reference> exposes its name when it is not 
> qualified (See 6.3 <table reference>, Syntax Rule 1).  Also, 
> an unqualified <table name> is equivalent to one qualified 
> with the current default schema name (See 5.4 Names and 
> identifiers, Syntax Rule 8).  So, in the above queries,
> select * from t1, app.t1  -- is the same as select * from 
> app.t1, app.t1, and this is not SQL92 --- you have duplicate 
> exposed <table name>s in the same scope (see 6.3 <table 
> reference>, Syntax Rule 3). Derby can support it, but it's an 
> extension.
> select t1.c1 from t1, app.t1 -- is the same as select app.t1.c1 
> from app.t1, app.t1, and again supporting this would be an 
> extension to SQL92.
> Note that if you say
> select * from t1, t2 t1 --  this is also a duplication, as the 
> exposed <table name> of the first table is app.t1 and the 
> exposed <correlation name> of the second table is t1. these 
> are different names, but 6.3 <table reference> Syntax Rule 4 
> rules out this case explicitly -- a <correlation name> 
> cannot be the same as the unqualified part of any exposed 
> <table name>.
> One possibility is to go to a 2 pass method of column resolution:
> 1st pass looks for an exact match on the qualifier (app.t1 
> matches app.t1 but not t1, ...)
> if no match, then do 2nd pass where it looks for a match on 
> table id.
> (Look at OrderByColumn.bindOrderByColumn()

-- 
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
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


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


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 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 Shreyas Kaushik <Sh...@Sun.COM>.
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-----
>>
>>  
>>

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

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:

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

Thus the patch needs to be careful of this overloaded use of TableName.
Does your patch handle this correctly? 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.

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


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,

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.
By calling this proper schema names get stored. I had to do these 
changes in some of the file where the method
getMatchingColumn() is called so that TableName comparisions happen on 
objects that have the correct data.

When put this patch and ran the tests I had some failures which were 
becasue the error messages were different.
This is because in the earlier file, if the error message had "S1.T1" 
now it has "APP.S1.T1" hence the failures.

I am attaching the patch and the test run results.

thanks
Shreyas

Daniel John Debrunner wrote:

>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>Shreyas Kaushik (JIRA) wrote:
>  
>
>>     [
>>    
>>
>http://issues.apache.org/jira/browse/DERBY-18?page=comments#action_58008 ]
>  
>
>>Shreyas Kaushik commented on DERBY-18:
>>--------------------------------------
>>
>>set current schema app;
>>
>>create table t1(int c1 varchar(10));
>>
>>create schema s1;
>>create table s1.t1(id1 int, d2 varchar(10));
>>
>>select * from t1, app.t1;  ---> This fails, should succeed.
>>
>>Internally table names are handled correctly but the above problem
>>    
>>
>occurs because of the equals() method in Tablename.java.
>  
>
>>Here the follwoing piece of code
>>
>>
>>    
>>
>-
>----------------------------------------------------------------------------------
>  
>
>>else if ((schemaName == null) ||
>>				 (otherTableName.getSchemaName() == null))
>>		{
>>			return tableName.equals(otherTableName.getTableName());
>>	     }
>>
>>    
>>
>-
>----------------------------------------------------------------------------------
>  
>
>>causes the above error to occur. In this if either of the schema name
>>    
>>
>is null, only the table names are compared and hence the error for the
>above statement.
>  
>
>>Should the equals method implementation be changed to take of this?
>>  or
>>Should we set the tables that have null schemas to the current schema
>>    
>>
>or something along these lines?
>
>Any implicit schema always means the current schema. At bind time the
>query node should ensure that it converts implicit schema names to the
>current schema. This is handled by the
>QueryTreeNode.getSchemaDescriptor() methods. Most likely this is
>occuring but the node's TableName is not being updated.
>
>Dan.
>-----BEGIN PGP SIGNATURE-----
>Version: GnuPG v1.2.5 (MingW32)
>Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
>iD8DBQFB9qR9Iv0S4qsbfuQRAvFMAJ4+Ujvl1x8Yp9DL9zFgHO8KnxQNiwCg0H6A
>HJ2QJcOVQncp5gYEVrKG7C8=
>=SZnN
>-----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 (JIRA) wrote:
>      [
http://issues.apache.org/jira/browse/DERBY-18?page=comments#action_58008 ]
>
> Shreyas Kaushik commented on DERBY-18:
> --------------------------------------
>
> set current schema app;
>
> create table t1(int c1 varchar(10));
>
> create schema s1;
> create table s1.t1(id1 int, d2 varchar(10));
>
> select * from t1, app.t1;  ---> This fails, should succeed.
>
> Internally table names are handled correctly but the above problem
occurs because of the equals() method in Tablename.java.
>
> Here the follwoing piece of code
>
>
-
----------------------------------------------------------------------------------
> else if ((schemaName == null) ||
> 				 (otherTableName.getSchemaName() == null))
> 		{
> 			return tableName.equals(otherTableName.getTableName());
> 	     }
>
-
----------------------------------------------------------------------------------
>
> causes the above error to occur. In this if either of the schema name
is null, only the table names are compared and hence the error for the
above statement.
>
> Should the equals method implementation be changed to take of this?
>   or
> Should we set the tables that have null schemas to the current schema
or something along these lines?

Any implicit schema always means the current schema. At bind time the
query node should ensure that it converts implicit schema names to the
current schema. This is handled by the
QueryTreeNode.getSchemaDescriptor() methods. Most likely this is
occuring but the node's TableName is not being updated.

Dan.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFB9qR9Iv0S4qsbfuQRAvFMAJ4+Ujvl1x8Yp9DL9zFgHO8KnxQNiwCg0H6A
HJ2QJcOVQncp5gYEVrKG7C8=
=SZnN
-----END PGP SIGNATURE-----