Derby
  1. Derby
  2. DERBY-5779

Table functions should only accept arguments which are constant in their query block.

    Details

    • Urgency:
      Normal
    • Issue & fix info:
      Patch Available, Release Note Needed
    • Bug behavior facts:
      Deviation from standard, Wrong query result

      Description

      Derby lets you invoke a table function in the FROM list of a query, passing in arguments built out of columns in other tables in the FROM list. This syntax is illegal and the resulting queries have no meaning under the SQL Standard. See the discussion on DERBY-5554. We should forbid this syntax. Similar syntax involving correlated subqueries in the FROM list is already forbidden. Fixing this will create a backward incompatibility which requires a release note.

      1. derby-5779-01-ab-forbidReferencesInQueryBlock.diff
        8 kB
        Rick Hillegas
      2. derby-5779-02-aa-forbidReferencesToJoinedTables.diff
        5 kB
        Rick Hillegas
      3. derby-5779-03-aa-moreTests.diff
        5 kB
        Rick Hillegas
      4. derby-5779-04-aa-subqueriesInFromList.diff
        9 kB
        Rick Hillegas
      5. derby-5779-04-ab-subqueriesInFromList.diff
        18 kB
        Rick Hillegas
      6. releaseNote.html
        3 kB
        Rick Hillegas

        Issue Links

          Activity

          Rick Hillegas created issue -
          Rick Hillegas made changes -
          Field Original Value New Value
          Link This issue relates to DERBY-5554 [ DERBY-5554 ]
          Hide
          Rick Hillegas added a comment -

          Derby relies on the illegal syntax for some of its internal metadata queries. For instance, the getFunctionColumns() query (in metadata.properties) involves a join, in the FROM list, between a VTI's arguments and columns from other tables in the FROM list. This raises the following red flags:

          1) Fixing the syntax may involve re-writing existing Derby metadata queries.

          2) The backward compatibility problem may be too big to contemplate.

          We may want to narrow the scope of this issue and just fix the problem for table functions, leaving Derby VTIs alone. We could probably back ourselves into the position that VTIs fall outside the SQL Standard. As a Derby extension, we can make up our own rules for them.

          The following phased approach might make sense:

          A) Add new overloads so that Derby table functions (like SYSCS_DIAG.SPACE_TABLE) can be invoked in a legal way.

          B) Change the documentation accordingly so that we no longer showcase illegal syntax.

          C) Make the bad syntax illegal for table functions, but leave VTIs alone.

          Later on, we may want to give some thought to what the illegal VTI syntax means. It may have a well-defined meaning provided that we can guarantee the join order which Derby chooses.

          Show
          Rick Hillegas added a comment - Derby relies on the illegal syntax for some of its internal metadata queries. For instance, the getFunctionColumns() query (in metadata.properties) involves a join, in the FROM list, between a VTI's arguments and columns from other tables in the FROM list. This raises the following red flags: 1) Fixing the syntax may involve re-writing existing Derby metadata queries. 2) The backward compatibility problem may be too big to contemplate. We may want to narrow the scope of this issue and just fix the problem for table functions, leaving Derby VTIs alone. We could probably back ourselves into the position that VTIs fall outside the SQL Standard. As a Derby extension, we can make up our own rules for them. The following phased approach might make sense: A) Add new overloads so that Derby table functions (like SYSCS_DIAG.SPACE_TABLE) can be invoked in a legal way. B) Change the documentation accordingly so that we no longer showcase illegal syntax. C) Make the bad syntax illegal for table functions, but leave VTIs alone. Later on, we may want to give some thought to what the illegal VTI syntax means. It may have a well-defined meaning provided that we can guarantee the join order which Derby chooses.
          Hide
          Dag H. Wanvik added a comment -

          Sounds like a good plan to me. As long as we can't express the intent of the query in another way (maybe recursive SQL would enable it?), and we can define the semantics precisely, I guess we could retain the syntax for VTIs.

          Show
          Dag H. Wanvik added a comment - Sounds like a good plan to me. As long as we can't express the intent of the query in another way (maybe recursive SQL would enable it?), and we can define the semantics precisely, I guess we could retain the syntax for VTIs.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-5779-01-ab-forbidReferencesInQueryBlock.diff. This patch prevents table function parameters from referring to other tables in the same query block. I am running regression tests now.

          There already was a place in the bind() logic for table functions where we dug up column references inside parameter expressions. I simply inserted another check there.

          Touches the following files:

          ---------

          M java/engine/org/apache/derby/loc/messages.xml
          M java/shared/org/apache/derby/shared/common/reference/SQLState.java

          Added a new error message for this condition. I considered re-using 42X04, which we use for the parallel error with correlated subqueries in the FROM list. However, I think that message is already too long, complicated, and confusing.

          ---------

          M java/engine/org/apache/derby/impl/sql/compile/FromVTI.java

          The actual fix.

          ---------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java

          New tests.

          Show
          Rick Hillegas added a comment - Attaching derby-5779-01-ab-forbidReferencesInQueryBlock.diff. This patch prevents table function parameters from referring to other tables in the same query block. I am running regression tests now. There already was a place in the bind() logic for table functions where we dug up column references inside parameter expressions. I simply inserted another check there. Touches the following files: --------- M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java Added a new error message for this condition. I considered re-using 42X04, which we use for the parallel error with correlated subqueries in the FROM list. However, I think that message is already too long, complicated, and confusing. --------- M java/engine/org/apache/derby/impl/sql/compile/FromVTI.java The actual fix. --------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java New tests.
          Rick Hillegas made changes -
          Rick Hillegas made changes -
          Assignee Rick Hillegas [ rhillegas ]
          Rick Hillegas made changes -
          Issue & fix info Release Note Needed [ 10101 ] Patch Available,Release Note Needed [ 10102, 10101 ]
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-5779-01-ab-forbidReferencesInQueryBlock.diff. Committed at subversion revision 1352631.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-5779-01-ab-forbidReferencesInQueryBlock.diff. Committed at subversion revision 1352631.
          Rick Hillegas made changes -
          Summary Table functions and vtis should only accept arguments which are constant in their query block. Table functions should only accept arguments which are constant in their query block.
          Description Derby lets you invoke a table function/vti in the FROM list of a query, passing in arguments built out of columns in other tables in the FROM list. This syntax is illegal and the resulting queries have no meaning under the SQL Standard. See the discussion on DERBY-5554. We should forbid this syntax. Similar syntax involving correlated subqueries in the FROM list is already forbidden. Fixing this will create a backward incompatibility which requires a release note. In particular, the sample usage for SYSCS_DIAG.SPACE_TABLE, given in the Reference Manual, will no longer work after fixing this bug. Changes to SPACE_TABLE are being discussed on DERBY-5554. Derby lets you invoke a table function in the FROM list of a query, passing in arguments built out of columns in other tables in the FROM list. This syntax is illegal and the resulting queries have no meaning under the SQL Standard. See the discussion on DERBY-5554. We should forbid this syntax. Similar syntax involving correlated subqueries in the FROM list is already forbidden. Fixing this will create a backward incompatibility which requires a release note.
          Hide
          Rick Hillegas added a comment -

          Attaching the first rev of a release note for this issue.

          Show
          Rick Hillegas added a comment - Attaching the first rev of a release note for this issue.
          Rick Hillegas made changes -
          Attachment releaseNote.html [ 12532931 ]
          Hide
          Rick Hillegas added a comment -

          Resolving this issue. Because of the scope of the backward incompatibility, I think that we should not port this fix to older release branches.

          Show
          Rick Hillegas added a comment - Resolving this issue. Because of the scope of the backward incompatibility, I think that we should not port this fix to older release branches.
          Rick Hillegas made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 10.10.0.0 [ 12321550 ]
          Resolution Fixed [ 1 ]
          Hide
          Rick Hillegas added a comment -

          Re-opening this issue. Another variant of this problem occurs when table functions and diagnostic VTIs are factors in a <joined table> expression and they take arguments built out of columns from other tables in the <joined table> expression. This is another example of violating part 2 of the SQL Standard, section 7.6 (<table reference>), Syntax Rule 6.a. Right now these queries do not even run. They raise NPEs and compiler assertions. We should raise a proper exception when trying to compile this illegal syntax. The following script shows how the bad syntax currently results in NPEs and compiler assertions:

          connect 'jdbc:derby:memory:db;create=true';

          create function lowerCaseRow( contents varchar( 32672 ) )
          returns table
          (
          contents varchar( 32672 )
          )
          language java parameter style DERBY_JDBC_RESULT_SET no sql
          external name 'org.apache.derbyTesting.functionTests.tests.lang.TableFunctionTest.lowerCaseRow';

          – NPEs
          select tt.* from table(syscs_diag.space_table(st.tablename)) tt join sys.systables st using(tableid);
          select tt.* from table( lowerCaseRow(st.tablename)) tt join sys.systables st on tt.contents = st.tablename;

          – Compiler assertions
          select tt.* from table(syscs_diag.space_table(st.tablename)) tt right join sys.systables st using(tableid);
          select tt.* from table( lowerCaseRow(st.tablename)) tt right join sys.systables st on tt.contents = st.tablename;

          Show
          Rick Hillegas added a comment - Re-opening this issue. Another variant of this problem occurs when table functions and diagnostic VTIs are factors in a <joined table> expression and they take arguments built out of columns from other tables in the <joined table> expression. This is another example of violating part 2 of the SQL Standard, section 7.6 (<table reference>), Syntax Rule 6.a. Right now these queries do not even run. They raise NPEs and compiler assertions. We should raise a proper exception when trying to compile this illegal syntax. The following script shows how the bad syntax currently results in NPEs and compiler assertions: connect 'jdbc:derby:memory:db;create=true'; create function lowerCaseRow( contents varchar( 32672 ) ) returns table ( contents varchar( 32672 ) ) language java parameter style DERBY_JDBC_RESULT_SET no sql external name 'org.apache.derbyTesting.functionTests.tests.lang.TableFunctionTest.lowerCaseRow'; – NPEs select tt.* from table(syscs_diag.space_table(st.tablename)) tt join sys.systables st using(tableid); select tt.* from table( lowerCaseRow(st.tablename)) tt join sys.systables st on tt.contents = st.tablename; – Compiler assertions select tt.* from table(syscs_diag.space_table(st.tablename)) tt right join sys.systables st using(tableid); select tt.* from table( lowerCaseRow(st.tablename)) tt right join sys.systables st on tt.contents = st.tablename;
          Rick Hillegas made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-5779-02-aa-forbidReferencesToJoinedTables.diff. This patch makes it illegal to join VTI arguments to other tables in <joined table> clauses. I will run regression tests.

          Touches the following files:

          ---------

          M java/engine/org/apache/derby/impl/sql/compile/FromVTI.java

          Continue to refine the logic in FromVTI.bindExpressions() in order to forbid these additional cases.

          ---------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java

          Add more test cases.

          Show
          Rick Hillegas added a comment - Attaching derby-5779-02-aa-forbidReferencesToJoinedTables.diff. This patch makes it illegal to join VTI arguments to other tables in <joined table> clauses. I will run regression tests. Touches the following files: --------- M java/engine/org/apache/derby/impl/sql/compile/FromVTI.java Continue to refine the logic in FromVTI.bindExpressions() in order to forbid these additional cases. --------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java Add more test cases.
          Rick Hillegas made changes -
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-5779-02-aa-forbidReferencesToJoinedTables.diff.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-5779-02-aa-forbidReferencesToJoinedTables.diff.
          Hide
          Rick Hillegas added a comment -

          Committed derby-5779-02-aa-forbidReferencesToJoinedTables.diff at subversion revision 1360736.

          Show
          Rick Hillegas added a comment - Committed derby-5779-02-aa-forbidReferencesToJoinedTables.diff at subversion revision 1360736.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-5779-03-aa-moreTests.diff. This adds tests for more <joined table> cases. Committed at subversion revision 1360846.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java

          Show
          Rick Hillegas added a comment - Attaching derby-5779-03-aa-moreTests.diff. This adds tests for more <joined table> cases. Committed at subversion revision 1360846. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java
          Rick Hillegas made changes -
          Attachment derby-5779-03-aa-moreTests.diff [ 12536254 ]
          Hide
          Rick Hillegas added a comment -

          Another case which needs to be forbidden:

          Derby trips over a compiler assertion if an argument to a VTI/tableFunction in a subquery in the FROM list references another table in the FROM list. The following script shows this:

          connect 'jdbc:derby:memory:db;create=true';

          create function lowerCaseRow( contents varchar( 32672 ) )
          returns table
          (
          contents varchar( 32672 )
          )
          language java parameter style DERBY_JDBC_RESULT_SET no sql
          external name 'org.apache.derbyTesting.functionTests.tests.lang.TableFunctionTest.lowerCaseRow';

          create table t1 (a int);

          – ok
          select tt.*
          from
          sys.systables systabs,
          ( select * from table (syscs_diag.space_table( 'T1' )) as t2 ) tt
          where systabs.tabletype = 'T' and systabs.tableid = tt.tableid;
          select tt.*
          from
          sys.systables systabs,
          ( select * from table (lowerCaseRow( 'T1' )) as t2 ) tt
          where systabs.tabletype = 'T' and systabs.tablename = tt.contents;

          – raises compiler assertions
          select tt.*
          from
          sys.systables systabs,
          ( select * from table (syscs_diag.space_table( systabs.tablename )) as t2 ) tt
          where systabs.tabletype = 'T' and systabs.tableid = tt.tableid;
          select tt.*
          from
          sys.systables systabs,
          ( select * from table (lowerCaseRow( systabs.tablename )) as t2 ) tt
          where systabs.tabletype = 'T' and systabs.tablename = tt.contents;

          Show
          Rick Hillegas added a comment - Another case which needs to be forbidden: Derby trips over a compiler assertion if an argument to a VTI/tableFunction in a subquery in the FROM list references another table in the FROM list. The following script shows this: connect 'jdbc:derby:memory:db;create=true'; create function lowerCaseRow( contents varchar( 32672 ) ) returns table ( contents varchar( 32672 ) ) language java parameter style DERBY_JDBC_RESULT_SET no sql external name 'org.apache.derbyTesting.functionTests.tests.lang.TableFunctionTest.lowerCaseRow'; create table t1 (a int); – ok select tt.* from sys.systables systabs, ( select * from table (syscs_diag.space_table( 'T1' )) as t2 ) tt where systabs.tabletype = 'T' and systabs.tableid = tt.tableid; select tt.* from sys.systables systabs, ( select * from table (lowerCaseRow( 'T1' )) as t2 ) tt where systabs.tabletype = 'T' and systabs.tablename = tt.contents; – raises compiler assertions select tt.* from sys.systables systabs, ( select * from table (syscs_diag.space_table( systabs.tablename )) as t2 ) tt where systabs.tabletype = 'T' and systabs.tableid = tt.tableid; select tt.* from sys.systables systabs, ( select * from table (lowerCaseRow( systabs.tablename )) as t2 ) tt where systabs.tabletype = 'T' and systabs.tablename = tt.contents;
          Hide
          Rick Hillegas added a comment -

          Attaching derby-5779-04-aa-subqueriesInFromList.diff. This is the first rev of a patch to forbid the problem identified above with subqueries in the FROM list. This patch is not ready for commit. I need to add more test cases and run regression tests.

          Show
          Rick Hillegas added a comment - Attaching derby-5779-04-aa-subqueriesInFromList.diff. This is the first rev of a patch to forbid the problem identified above with subqueries in the FROM list. This patch is not ready for commit. I need to add more test cases and run regression tests.
          Rick Hillegas made changes -
          Hide
          Rick Hillegas added a comment -

          Attaching derby-5779-04-ab-subqueriesInFromList.diff. Second rev of patch to fix correlated references to outer tables by arguments to VTIs/tableFunctions invoked inside subqueries. I am running regression tests now.

          Touches the following files:

          ----------

          M java/engine/org/apache/derby/impl/sql/compile/FromSubquery.java

          At bind time, a subquery in the FROM list now pokes the FROM list into any VTIs invoked inside it.

          ----------

          M java/engine/org/apache/derby/impl/sql/compile/FromVTI.java

          When binding VTI arguments, we check to see if they contain correlated references to outer query blocks. If so, we make sure that those references are not to tables on the FROM lists which were poked into the VTI by outer subqueries.

          ----------

          M java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java

          We hit an NPE if we permute the join order, putting the subquery in the FROM list ahead of the tables referenced by args in its nested VTIs. We replace that NPE with an error message saying that we have detected an illegal reference in a VTI argument.

          ----------

          M java/engine/org/apache/derby/loc/messages.xml

          We reword the 42ZB7 message to reflect the fact that it is being raised in more situations now.

          ----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/SysDiagVTIMappingTest.java

          Additional test cases.

          Show
          Rick Hillegas added a comment - Attaching derby-5779-04-ab-subqueriesInFromList.diff. Second rev of patch to fix correlated references to outer tables by arguments to VTIs/tableFunctions invoked inside subqueries. I am running regression tests now. Touches the following files: ---------- M java/engine/org/apache/derby/impl/sql/compile/FromSubquery.java At bind time, a subquery in the FROM list now pokes the FROM list into any VTIs invoked inside it. ---------- M java/engine/org/apache/derby/impl/sql/compile/FromVTI.java When binding VTI arguments, we check to see if they contain correlated references to outer query blocks. If so, we make sure that those references are not to tables on the FROM lists which were poked into the VTI by outer subqueries. ---------- M java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java We hit an NPE if we permute the join order, putting the subquery in the FROM list ahead of the tables referenced by args in its nested VTIs. We replace that NPE with an error message saying that we have detected an illegal reference in a VTI argument. ---------- M java/engine/org/apache/derby/loc/messages.xml We reword the 42ZB7 message to reflect the fact that it is being raised in more situations now. ---------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/TableFunctionTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/SysDiagVTIMappingTest.java Additional test cases.
          Rick Hillegas made changes -
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly on derby-5779-04-ab-subqueriesInFromList.diff. Committed at subversion revision 1362159.

          Show
          Rick Hillegas added a comment - Tests passed cleanly on derby-5779-04-ab-subqueriesInFromList.diff. Committed at subversion revision 1362159.
          Hide
          Rick Hillegas added a comment -

          Resolving this issue again. We can re-open the issue if we discover other instances of this bad syntax.

          Show
          Rick Hillegas added a comment - Resolving this issue again. We can re-open the issue if we discover other instances of this bad syntax.
          Rick Hillegas made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Rick Hillegas made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Mike Matrigali made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Hide
          Mike Matrigali added a comment -

          fix adds a backword incompatibility so should not be backported.

          Show
          Mike Matrigali added a comment - fix adds a backword incompatibility so should not be backported.
          Mike Matrigali made changes -
          Labels derby_backport_reject_10_9
          Mike Matrigali made changes -
          Status Reopened [ 4 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Gavin made changes -
          Workflow jira [ 12669119 ] Default workflow, editable Closed status [ 12802653 ]

            People

            • Assignee:
              Rick Hillegas
              Reporter:
              Rick Hillegas
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development