db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shreyas Kaushik <Shreyas.Kaus...@Sun.COM>
Subject Re: [PATCH] Derby-156
Date Wed, 01 Jun 2005 11:28:37 GMT
Did anyone get a chance to review this?

thanks
Shreyas

Shreyas Kaushik wrote:

> Attached is the latest patch. Ran derbylang suite successfully without 
> any failures.
>
> ~ Shreyas
>
> Satheesh Bandaram wrote:
>
>> Is this patch ready for review? Still any pending issues here?
>>
>> Satheesh
>>
>> Shreyas Kaushik wrote:
>>
>>  
>>
>>> Hi Dan,
>>>
>>> If you are ok with this, I can add the comments to the patch and send
>>> it out on the alias.
>>>
>>> thanks
>>> Shreyas
>>>
>>> Shreyas Kaushik wrote:
>>>
>>>   
>>>
>>>> Ok. Thanks for your suggestions I will do it.
>>>>
>>>> The idea behibd adding that peice of code is:
>>>>
>>>> The *delete from* did not have support for handling correlation
>>>> names. So when we are creating a fresh ResultColumnList
>>>> and the FromBaseTable we need to pass the correlation name to this
>>>> newly created FromBaseTable object from the target table where it
>>>> will stored.
>>>>
>>>> thanks
>>>> Shreyas
>>>>
>>>> Daniel John Debrunner wrote:
>>>>
>>>>     
>>>>
>>>>> Shreyas Kaushik wrote:
>>>>>
>>>>>
>>>>>       
>>>>>
>>>>>> Did anyone gfo through this?
>>>>>>  
>>>>>>         
>>>>>
>>>>>
>>>>> I looked briefly. Do you have an explanation of your changes?
>>>>> Adding comments to the code you added in DeleteNode, explaining what
>>>>> exactly that new block is doing would be really useful.
>>>>>
>>>>> I would say the chance of a fix being committed quickly increases
>>>>> significantly if it is well explained and well commented. The code
>>>>> changes really need to stand alone, ie. be understandable through
>>>>> comments, as anyone looking through the code in the future will not
>>>>> have
>>>>> a link to any e-mail discussion to help them along. Though a 
>>>>> summary of
>>>>> the changes with the contribution can be useful. Such a summary is 
>>>>> also
>>>>> a useful commit comment for the svn history of changes to the 
>>>>> codeline.
>>>>>
>>>>> Writing code comments is also very helpful to the writer of the code,
>>>>> helps to cement the mental ideas into code. A good practise is to 
>>>>> write
>>>>> the comments first and then the code.
>>>>>
>>>>> Dan.
>>>>>
>>>>>
>>>>>
>>>>>       
>>>>>
>>>>>>>>> Index:
>>>>>>>>> java/engine/org/apache/derby/impl/sql/compile/DeleteNode.java
>>>>>>>>> ===================================================================

>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>> java/engine/org/apache/derby/impl/sql/compile/DeleteNode.java
    
>>>>>>>>> (revision 161449)
>>>>>>>>> +++
>>>>>>>>> java/engine/org/apache/derby/impl/sql/compile/DeleteNode.java
    
>>>>>>>>> (working copy)
>>>>>>>>> @@ -241,6 +241,13 @@
>>>>>>>>>                     resultColumnList = new ResultColumnList();
>>>>>>>>>
>>>>>>>>>                     FromBaseTable fbt =
>>>>>>>>> getResultColumnList(resultColumnList);
>>>>>>>>> +
>>>>>>>>> +                        if(targetTable instanceof 
>>>>>>>>> FromBaseTable) {
>>>>>>>>> +                            String correlationName;
>>>>>>>>> +                            correlationName =
>>>>>>>>> ((FromBaseTable)targetTable).correlationName;
>>>>>>>>> +                            if(correlationName != null)
>>>>>>>>> +                                fbt.correlationName
=
>>>>>>>>> correlationName;
>>>>>>>>> +                        }
>>>>>>>>>
>>>>>>>>>                     readColsBitSet = getReadMap(dataDictionary,
>>>>>>>>>                                                     
                     
>>>>>>>>> targetTableDescriptor);
>>>>>>>>>                      
>>>>>>>>
>>>>>>>>             
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>       
>>>>
>>>
>>>   
>>
>>
>>  
>>
>------------------------------------------------------------------------
>
>Index: java/engine/org/apache/derby/impl/sql/compile/DeleteNode.java
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/DeleteNode.java	(revision 178358)
>+++ java/engine/org/apache/derby/impl/sql/compile/DeleteNode.java	(working copy)
>@@ -241,6 +241,18 @@
> 			resultColumnList = new ResultColumnList();
> 
> 			FromBaseTable fbt = getResultColumnList(resultColumnList);
>+                       
>+                        /* When we are creating a fresh ResultColumnList
>+                         * and the FromBaseTable we need to pass the correlation name

>+                         * to this newly created FromBaseTable object from the target
table 
>+                         * where it will be stored.
>+                         */
>+                        if(targetTable instanceof FromBaseTable) {
>+                            String correlationName;
>+                            correlationName = ((FromBaseTable)targetTable).correlationName;
>+                            if(correlationName != null)
>+                                fbt.correlationName = correlationName;
>+                        }
> 
> 			readColsBitSet = getReadMap(dataDictionary,
> 										targetTableDescriptor);
>Index: java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
>===================================================================
>--- java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj	(revision 178358)
>+++ java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj	(working copy)
>@@ -2582,16 +2582,27 @@
> 	QueryTreeNode retval;
> 	Properties targetProperties = null;
> 	Token	   whereToken = null;
>+        String correlationName = null;
>+        Object []objArr = null;
> }
> {
> 	LOOKAHEAD( { fromNewInvocationFollows() } ) 
> 	<FROM> javaToSQLNode = newInvocation()
>+	/**
>+	  * Get the correlation name if there is any in the SQL statement
>+	  * Extract the correlation name and pass it on to the FromTable Node
>+	  * for binding
>+	  */
>+        {
>+           objArr = optionalTableClauses();
>+           correlationName = (String)objArr[OPTIONAL_TABLE_CLAUSES_CORRELATION_NAME];
>+        }
> 	[ whereToken = <WHERE> whereClause = whereClause(whereToken) ]
> 	{
> 		fromTable =  (FromTable) nodeFactory.getNode(
> 									C_NodeTypes.FROM_VTI,
> 									javaToSQLNode.getJavaValueNode(), 
>-									(String) null,
>+									correlationName,
> 									null, 
> 									(Properties) null,
> 									getContextManager()); 
>@@ -2600,6 +2611,15 @@
> 	}
> |
> 	<FROM> tableName = qualifiedName(Limits.MAX_IDENTIFIER_LENGTH) 
>+	/**
>+	  * Get the correlation name if there is any in the SQL statement
>+	  * Extract the correlation name and pass it on to the FromTable Node
>+	  * for binding
>+	  */
>+        {
>+           objArr = optionalTableClauses();
>+           correlationName = (String)objArr[OPTIONAL_TABLE_CLAUSES_CORRELATION_NAME];
>+        }
> 	    [targetProperties = propertyList() ] 
> 		[ 
> 			whereToken = <WHERE>
>@@ -2631,7 +2651,7 @@
> 			fromTable = (FromTable) nodeFactory.getNode(
> 										C_NodeTypes.FROM_BASE_TABLE,
> 										tableName,
>-										null,
>+										correlationName,
> 										ReuseFactory.getInteger(
> 												FromBaseTable.DELETE),
> 										null,
>Index: java/testing/org/apache/derbyTesting/functionTests/tests/lang/corrDelete.sql
>===================================================================
>--- java/testing/org/apache/derbyTesting/functionTests/tests/lang/corrDelete.sql	(revision
0)
>+++ java/testing/org/apache/derbyTesting/functionTests/tests/lang/corrDelete.sql	(revision
0)
>@@ -0,0 +1,24 @@
>+-- This tests the delete functionality with correlation name
>+
>+create table corrDelete(ival int, cval varchar(10));
>+insert into corrDelete values(1,'test1');
>+insert into corrDelete values(2,'test2');
>+insert into corrDelete values(3,'test3');
>+insert into corrDelete values(4,'test4');
>+insert into corrDelete values(5,'test5');
>+insert into corrDelete values(6,'test6');
>+
>+select * from corrDelete;
>+
>+delete from corrDelete d where ival=3;
>+
>+select * from corrDelete;
>+
>+delete from corrDelete as d where d.ival=5;
>+
>+select * from corrDelete;
>+
>+delete from corrDelete d;
>+
>+select * from corrDelete;
>+
>Index: java/testing/org/apache/derbyTesting/functionTests/tests/lang/refActions1.sql
>===================================================================
>--- java/testing/org/apache/derbyTesting/functionTests/tests/lang/refActions1.sql	(revision
178358)
>+++ java/testing/org/apache/derbyTesting/functionTests/tests/lang/refActions1.sql	(working
copy)
>@@ -719,6 +719,14 @@
> select * from db2test.emp13 order by dno, name, mgrname;
> select * from db2test.emp14 order by dno, name, mgrname;
> select * from db2test.emp15 order by dno, name, mgrname;
>+delete from db2test.dept d where
>+  dno in (select dno from db2test.emp e where
>+ e.dno = d.dno and e.dno in (select dno from db2test.emp2 e2 where
>+ e2.dno = e.dno and e2.dno in (select dno from db2test.emp3 e3 where
>+ e3.dno = e2.dno and e3.dno in (select dno from db2test.emp4 e4 where
>+ e4.dno = e3.dno and e4.dno in (select dno from db2test.emp5 e5 where
>+ e5.dno = e4.dno and e5.dno in (select dno from db2test.emp6 e6 where
>+ e6.dno = e5.dno and e6.dno in ('K55', 'K52')))))));
> -- "END OF TESTUNIT: 11";
> 
> 
>@@ -2306,14 +2314,6 @@
>  e5.dno = e4.dno and e5.dno in (select dno from db2test.emp6 e6 where
>  e6.dno = e5.dno and e6.dno in ('K55', 'K52')))))))
>  order by 2, 3;
>-delete from db2test.dept d where
>-  dno in (select dno from db2test.emp e where
>- e.dno = d.dno and e.dno in (select dno from db2test.emp2 e2 where
>- e2.dno = e.dno and e2.dno in (select dno from db2test.emp3 e3 where
>- e3.dno = e2.dno and e3.dno in (select dno from db2test.emp4 e4 where
>- e4.dno = e3.dno and e4.dno in (select dno from db2test.emp5 e5 where
>- e5.dno = e4.dno and e5.dno in (select dno from db2test.emp6 e6 where
>- e6.dno = e5.dno and e6.dno in ('K55', 'K52')))))));
> select * from db2test.dept order by dno, dname;
> select * from db2test.emp order by dno, name, mgrname;
> select * from db2test.secondemp order by dno, name, mgrname;
>Index: java/testing/org/apache/derbyTesting/functionTests/tests/lang/copyfiles.ant
>===================================================================
>--- java/testing/org/apache/derbyTesting/functionTests/tests/lang/copyfiles.ant	(revision
178358)
>+++ java/testing/org/apache/derbyTesting/functionTests/tests/lang/copyfiles.ant	(working
copy)
>@@ -209,3 +209,4 @@
> wisconsin_app.properties
> wisconsin_derby.properties
> wisconsin_sed.properties
>+corrDelete.sql
>Index: java/testing/org/apache/derbyTesting/functionTests/master/corrDelete.out
>===================================================================
>--- java/testing/org/apache/derbyTesting/functionTests/master/corrDelete.out	(revision
0)
>+++ java/testing/org/apache/derbyTesting/functionTests/master/corrDelete.out	(revision
0)
>@@ -0,0 +1,49 @@
>+ij> -- This tests the delete functionality with correlation name
>+create table corrDelete(ival int, cval varchar(10));
>+0 rows inserted/updated/deleted
>+ij> insert into corrDelete values(1,'test1');
>+1 row inserted/updated/deleted
>+ij> insert into corrDelete values(2,'test2');
>+1 row inserted/updated/deleted
>+ij> insert into corrDelete values(3,'test3');
>+1 row inserted/updated/deleted
>+ij> insert into corrDelete values(4,'test4');
>+1 row inserted/updated/deleted
>+ij> insert into corrDelete values(5,'test5');
>+1 row inserted/updated/deleted
>+ij> insert into corrDelete values(6,'test6');
>+1 row inserted/updated/deleted
>+ij> select * from corrDelete;
>+IVAL       |CVAL      
>+----------------------
>+1          |test1     
>+2          |test2     
>+3          |test3     
>+4          |test4     
>+5          |test5     
>+6          |test6     
>+ij> delete from corrDelete d where ival=3;
>+1 row inserted/updated/deleted
>+ij> select * from corrDelete;
>+IVAL       |CVAL      
>+----------------------
>+1          |test1     
>+2          |test2     
>+4          |test4     
>+5          |test5     
>+6          |test6     
>+ij> delete from corrDelete as d where d.ival=5;
>+1 row inserted/updated/deleted
>+ij> select * from corrDelete;
>+IVAL       |CVAL      
>+----------------------
>+1          |test1     
>+2          |test2     
>+4          |test4     
>+6          |test6     
>+ij> delete from corrDelete d;
>+4 rows inserted/updated/deleted
>+ij> select * from corrDelete;
>+IVAL       |CVAL      
>+----------------------
>+ij> 
>Index: java/testing/org/apache/derbyTesting/functionTests/master/refActions1.out
>===================================================================
>--- java/testing/org/apache/derbyTesting/functionTests/master/refActions1.out	(revision
178358)
>+++ java/testing/org/apache/derbyTesting/functionTests/master/refActions1.out	(working
copy)
>@@ -1733,6 +1733,15 @@
> 5          |JOE2      |ASHOK     |K51 
> 2          |JOHN      |ASHOK     |K51 
> 3          |ROBIN     |ASHOK     |K51 
>+ij> delete from db2test.dept d where
>+  dno in (select dno from db2test.emp e where
>+ e.dno = d.dno and e.dno in (select dno from db2test.emp2 e2 where
>+ e2.dno = e.dno and e2.dno in (select dno from db2test.emp3 e3 where
>+ e3.dno = e2.dno and e3.dno in (select dno from db2test.emp4 e4 where
>+ e4.dno = e3.dno and e4.dno in (select dno from db2test.emp5 e5 where
>+ e5.dno = e4.dno and e5.dno in (select dno from db2test.emp6 e6 where
>+ e6.dno = e5.dno and e6.dno in ('K55', 'K52')))))));
>+0 rows inserted/updated/deleted
> ij> -- "END OF TESTUNIT: 11";
> -- *************************************************************************
> -- TESTUNIT         : 12
>@@ -5998,15 +6007,6 @@
> --------------------------
> 2          |K52|OFC       
> 1          |K55|DB        
>-ij> delete from db2test.dept d where
>-  dno in (select dno from db2test.emp e where
>- e.dno = d.dno and e.dno in (select dno from db2test.emp2 e2 where
>- e2.dno = e.dno and e2.dno in (select dno from db2test.emp3 e3 where
>- e3.dno = e2.dno and e3.dno in (select dno from db2test.emp4 e4 where
>- e4.dno = e3.dno and e4.dno in (select dno from db2test.emp5 e5 where
>- e5.dno = e4.dno and e5.dno in (select dno from db2test.emp6 e6 where
>- e6.dno = e5.dno and e6.dno in ('K55', 'K52')))))));
>-ERROR 42X01: Syntax error: Encountered "d" at line 1, column 26.
> ij> select * from db2test.dept order by dno, dname;
> C0         |DNO|DNAME     
> --------------------------
>@@ -7585,7 +7585,7 @@
>                   where e3.name = e2.mgrname group by dno having
>                   e2.dno in (select dno from db2test.emp  e1
>                   where e1.name = e.mgrname and e1.mgrname = 'JOHN')))));
>-ERROR 42X01: Syntax error: Encountered "e" at line 1, column 25.
>+ERROR 42X04: Column 'E5.NAME' 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 'E5.NAME' is not a column in the target table.
> ij> select * from db2test.emp order by dno, name, mgrname;
> C0         |NAME      |MGRNAME   |DNO 
> --------------------------------------
>@@ -7753,7 +7753,7 @@
>                   where e.name = e2.mgrname group by dno having
>                   e2.dno in (select dno from db2test.emp  e1
>                    where e.mgrname = 'JOHN')))));
>-ERROR 42X01: Syntax error: Encountered "e" at line 1, column 25.
>+ERROR 42X04: Column 'E.NAME' 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 'E.NAME' is not a column in the target table.
> ij> select * from db2test.emp order by dno, name, mgrname;
> C0         |NAME      |MGRNAME   |DNO 
> --------------------------------------
>Index: java/testing/org/apache/derbyTesting/functionTests/suites/derbylang.runall
>===================================================================
>--- java/testing/org/apache/derbyTesting/functionTests/suites/derbylang.runall	(revision
178358)
>+++ java/testing/org/apache/derbyTesting/functionTests/suites/derbylang.runall	(working
copy)
>@@ -31,6 +31,7 @@
> lang/connect.sql
> lang/consistencyChecker.sql
> lang/constantExpression.sql
>+lang/corrDelete.sql
> lang/currentSchema.sql
> lang/currentof.java
> lang/cursor.java
>  
>

Mime
View raw message