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() } ) > 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 = whereClause = whereClause(whereToken) ] > { > fromTable = (FromTable) nodeFactory.getNode( > C_NodeTypes.FROM_VTI, > javaToSQLNode.getJavaValueNode(), >- (String) null, >+ correlationName, > null, > (Properties) null, > getContextManager()); >@@ -2600,6 +2611,15 @@ > } > | > 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 = >@@ -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 > >