Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.0.2.1
    • Fix Version/s: 10.5.1.1
    • Component/s: SQL
    • Labels:
      None
    • Issue & fix info:
      Patch Available

      Description

      Satheesh has pointed out that generated columns, a SQL 2003 feature, would satisfy the performance requirements of Expression Indexes (bug 455). Generated columns may not be as elegant as Expression Indexes, but they are easier to implement. We would allow the following new kind of column definition in CREATE TABLE and ALTER TABLE statements:

      columnName GENERATED ALWAYS AS ( expression )

      If expression were an indexableExpression (as defined in bug 455), then we could create indexes on it. There is no work for the optimizer to do here. The Language merely has to compute the generated column at INSERT/UPDATE time.

      1. derby-481-00-aa-prototype.diff
        132 kB
        Rick Hillegas
      2. derby-481-01-aa-catalog.diff
        5 kB
        Rick Hillegas
      3. derby-481-02-aa-utilities.diff
        34 kB
        Rick Hillegas
      4. derby-481-03-aa-grammar.diff
        60 kB
        Rick Hillegas
      5. derby-481-04-aa-insert.diff
        48 kB
        Rick Hillegas
      6. derby-481-05-aa-update.diff
        51 kB
        Rick Hillegas
      7. derby-481-06-aa-genreferences.diff
        12 kB
        Rick Hillegas
      8. derby-481-07-aa-noSQLinRoutines.diff
        10 kB
        Rick Hillegas
      9. derby-481-07-ab-noSQLinRoutines.diff
        11 kB
        Rick Hillegas
      10. derby-481-08-aa-castToDeclaredType.diff
        19 kB
        Rick Hillegas
      11. derby-481-09-aa-dummyDefaults.diff
        3 kB
        Rick Hillegas
      12. derby-481-10-aa-foreignKeyActions.diff
        16 kB
        Rick Hillegas
      13. derby-481-11-aa-notNull.diff
        19 kB
        Rick Hillegas
      14. derby-481-12-aa-padding.diff
        4 kB
        Rick Hillegas
      15. derby-481-13-aa-alterDatatype.diff
        5 kB
        Rick Hillegas
      16. derby-481-14-ab-dropColumn.diff
        45 kB
        Rick Hillegas
      17. derby-481-15-aa-renameAndAddDefault.diff
        7 kB
        Rick Hillegas
      18. derby-481-16-aa-dropFunction.diff
        15 kB
        Rick Hillegas
      19. derby-481-17-aa-currentSchema.diff
        0.9 kB
        Rick Hillegas
      20. derby-481-18-aa-updatableResultSets.diff
        6 kB
        Rick Hillegas
      21. derby-481-19-aa-cleanup.diff
        11 kB
        Rick Hillegas
      22. derby-481-20-aa-cleanup.diff
        8 kB
        Rick Hillegas
      23. derby-481-21-aa-bulkImport.diff
        5 kB
        Rick Hillegas
      24. GeneratedColumns.html
        23 kB
        Rick Hillegas
      25. GeneratedColumns.html
        23 kB
        Rick Hillegas
      26. GeneratedColumns.html
        21 kB
        Rick Hillegas

        Issue Links

          Activity

          Gavin made changes -
          Workflow jira [ 12322860 ] Default workflow, editable Closed status [ 12802368 ]
          Gavin made changes -
          Link This issue is depended upon by DERBY-3959 [ DERBY-3959 ]
          Gavin made changes -
          Link This issue blocks DERBY-3959 [ DERBY-3959 ]
          Rick Hillegas made changes -
          Link This issue is related to DERBY-4831 [ DERBY-4831 ]
          Rick Hillegas made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Rick Hillegas made changes -
          Link This issue is related to DERBY-1257 [ DERBY-1257 ]
          Dag H. Wanvik made changes -
          Issue Type New Feature [ 2 ] Improvement [ 4 ]
          Kathey Marsden made changes -
          Link This issue relates to DERBY-4142 [ DERBY-4142 ]
          Myrna van Lunteren made changes -
          Fix Version/s 10.5.1.1 [ 12313771 ]
          Fix Version/s 10.5.0.0 [ 12313010 ]
          Myrna van Lunteren made changes -
          Fix Version/s 10.5.0.0 [ 12313010 ]
          Rick Hillegas made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Resolved [ 5 ]
          Hide
          Rick Hillegas added a comment -

          I believe that the work on this issue is done now that Kim has checked in the user documentation. Thanks, Kim!

          Show
          Rick Hillegas added a comment - I believe that the work on this issue is done now that Kim has checked in the user documentation. Thanks, Kim!
          Rick Hillegas made changes -
          Attachment derby-481-21-aa-bulkImport.diff [ 12396739 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-21-aa-bulkImport.diff, which adds tests for bulk import of generated columns. Committed at subversion revision 729329. Touches the following files:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml
          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/t_bi_1.dat

          Show
          Rick Hillegas added a comment - Attaching derby-481-21-aa-bulkImport.diff, which adds tests for bulk import of generated columns. Committed at subversion revision 729329. Touches the following files: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml A java/testing/org/apache/derbyTesting/functionTests/tests/lang/t_bi_1.dat
          Hide
          Rick Hillegas added a comment -

          Hi Dag. I think your interpretation is reasonable.

          Show
          Rick Hillegas added a comment - Hi Dag. I think your interpretation is reasonable.
          Hide
          Dag H. Wanvik added a comment -

          A possible answer may be that a column level constraints is defined as equivalent to
          table level constraints in section 11.4 SR 12. Since this is a syntactic rule, I guess we could
          argue that the standard sees column level constraints as a syntactic variation of a table level constraint,
          in which case the RESTRICT behavior discussed above would apply to it as well. What do you think?

          Show
          Dag H. Wanvik added a comment - A possible answer may be that a column level constraints is defined as equivalent to table level constraints in section 11.4 SR 12. Since this is a syntactic rule, I guess we could argue that the standard sees column level constraints as a syntactic variation of a table level constraint, in which case the RESTRICT behavior discussed above would apply to it as well. What do you think?
          Hide
          Dag H. Wanvik added a comment -

          Hmm indeed puzzling that distinction about the table vs column level constraint.
          So, if it is a table level constraint, we do not restrict in the single column case?

          Show
          Dag H. Wanvik added a comment - Hmm indeed puzzling that distinction about the table vs column level constraint. So, if it is a table level constraint, we do not restrict in the single column case?
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3964 [ DERBY-3964 ]
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3969 [ DERBY-3969 ]
          Hide
          Rick Hillegas added a comment -

          Hi Dag,

          Thanks for your second batch of comments on November 26. I agree that it seems silly for RESTRICT to prevent you from dropping a column which is constrained by a CHECK constraint that only mentions that column. I'm not sure that the language of section 11.18 addresses this silliness, though. In the example from our test code, the CHECK constraint seems to me to be a column level CHECK constraint, not a table level constraint--so the exception in syntax rule (5b) doesn't apply. This is puzzling though. I don't know why the CHECK constraint would block a RESTRICTed DROP when phrased at the column level but would not block the DROP when phrased at the table level.

          Show
          Rick Hillegas added a comment - Hi Dag, Thanks for your second batch of comments on November 26. I agree that it seems silly for RESTRICT to prevent you from dropping a column which is constrained by a CHECK constraint that only mentions that column. I'm not sure that the language of section 11.18 addresses this silliness, though. In the example from our test code, the CHECK constraint seems to me to be a column level CHECK constraint, not a table level constraint--so the exception in syntax rule (5b) doesn't apply. This is puzzling though. I don't know why the CHECK constraint would block a RESTRICTed DROP when phrased at the column level but would not block the DROP when phrased at the table level.
          Rick Hillegas made changes -
          Attachment derby-481-20-aa-cleanup.diff [ 12395039 ]
          Hide
          Rick Hillegas added a comment -

          Thanks for the great feedback, Dag! Attaching derby-481-20-aa-cleanup.diff, incorporating Dag's comments. Committed at subversion revision 722214.

          Show
          Rick Hillegas added a comment - Thanks for the great feedback, Dag! Attaching derby-481-20-aa-cleanup.diff, incorporating Dag's comments. Committed at subversion revision 722214.
          Hide
          Dag H. Wanvik added a comment - - edited

          Reviewed derby-481-14-ab-dropColumn. Thanks again Rick! The patch is
          good to commit. My notes:

          • AlterTableConstantAction.java:
          • The following fragment threw me off at first:

          > int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - cascadedDrops;
          > // can NOT drop a column if it is the only one in the table
          > if (sizeAfterCascadedDrops == 1)

          Surely,, since we are talkign about size after cascaded drops, th test
          should be against 0? But no, the quantity sizeAfterCascadedDrops does
          not include the original dropped column, so the code works, but is a
          bit misleading. Maybe do:

          int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - 1 - cascadedDrops;
          // Can not drop a column if it there would be no columns left:
          if (sizeAfterCascadedDrops == 0)

          • Maybe it would be informative to add a comment on the recursive call
            that it never exceeds 2 in depth since generated columns don't
            reference other generated columns.
          • The new error message LANG_CASCADED_GEN_COL_DROP doesn't appear to be
            used in the patch?

          Is the intention that it be used in this exception?

          throw StandardException.newException(
          SQLState.LANG_PROVIDER_HAS_DEPENDENT_OBJECT,
          dm.getActionString(DependencyManager.DROP_COLUMN),
          "THE LAST COLUMN " + columnName,
          "TABLE",
          td.getQualifiedName() );

          The new tests use LANG_PROVIDER_HAS_DEPENDENT_OBJECT (X0Y25), so
          perhaps LANG_CASCADED_GEN_COL_DROP is cruft?
          The wording of X0Y25 doesn't really precisely describe the problem,
          though, so a new error message would be nice, I think...

          • The string "THE LAST COLUMN " is not i18n safe.
          • If generated column were to be dropped indirectly, columnName could
            not appear to the user to be THE LAST COLUMN; I think the error
            should point out in such cases that, after cascade action, we would
            have 0 columns left. Perhaps the wording can be word-smithed to
            handle both cases well.
          • The error message wording of LANG_CASCADED_GEN_COL_DROP is also
            confusing to me:

          "Dropping column '

          {0}

          ' would orphan the generated column '

          {1}

          ' which
          references it. You must drop the generated column first.

          I interpret this to mean that if 0 is dropped, 1 would be the only
          column left and a computed column can obviously not be alone in a
          table. But the issue would be clearer explained by saying that after
          1 is removed by cascade, there would be no columns left. I also
          don't understand the point about dropping the generated column
          first? Dropping 0 after having dropped 1 wouldn't improve things
          unless another column were added in the meantime? Or maybe I am
          misunderstanding here.. getting late

          • GeneratedColumnsTest.java
          • The verification that an index on a generated column is actually
            dropped could be better, I think. Presently, you only verify that
            an insert previosly blocked by the index is legal after the dropping
            of the column. It would be good to inspect the dictionary to see
            that it is actually gone physically as well.
          • Under verification that constraints are dropped, I started
            wondering.. I think this expected error is not according to the
            standard:

          :
          create table t_dc_10( a int generated always as ( -b ) check ( a is not null ), b int, c int )"
          alter table t_dc_10 drop column a restrict // gives X0Y25
          :

          According to section 11.18, we have:

          "5) If RESTRICT is specified, then C shall not be referenced in any of the following:
          a) The <query expression> of any view descriptor.

          b) The <search condition> of any constraint descriptor other than a
          table constraint descriptor that contains references to no other column and
          *********************************************************************
          that is included in the table descriptor of T.

          c) The SQL routine body of any routine descriptor.

          d) Either an explicit trigger column list or a triggered action
          column set of any trigger descriptor.

          e) The generation expression of any column descriptor."

          It would seem to be that in this case we are covered under item 5b),
          the check constraint only references the dropped column "a," so
          RESTRICT should not block this dropping. This is not
          behavior particular to generated columns, so if you agree with my interpretation, we should
          file a JIRA for it.

          Show
          Dag H. Wanvik added a comment - - edited Reviewed derby-481-14-ab-dropColumn. Thanks again Rick! The patch is good to commit. My notes: AlterTableConstantAction.java: The following fragment threw me off at first: > int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - cascadedDrops; > // can NOT drop a column if it is the only one in the table > if (sizeAfterCascadedDrops == 1) Surely,, since we are talkign about size after cascaded drops, th test should be against 0? But no, the quantity sizeAfterCascadedDrops does not include the original dropped column, so the code works, but is a bit misleading. Maybe do: int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - 1 - cascadedDrops; // Can not drop a column if it there would be no columns left: if (sizeAfterCascadedDrops == 0) Maybe it would be informative to add a comment on the recursive call that it never exceeds 2 in depth since generated columns don't reference other generated columns. The new error message LANG_CASCADED_GEN_COL_DROP doesn't appear to be used in the patch? Is the intention that it be used in this exception? throw StandardException.newException( SQLState.LANG_PROVIDER_HAS_DEPENDENT_OBJECT, dm.getActionString(DependencyManager.DROP_COLUMN), "THE LAST COLUMN " + columnName, "TABLE", td.getQualifiedName() ); The new tests use LANG_PROVIDER_HAS_DEPENDENT_OBJECT (X0Y25), so perhaps LANG_CASCADED_GEN_COL_DROP is cruft? The wording of X0Y25 doesn't really precisely describe the problem, though, so a new error message would be nice, I think... The string "THE LAST COLUMN " is not i18n safe. If generated column were to be dropped indirectly, columnName could not appear to the user to be THE LAST COLUMN; I think the error should point out in such cases that, after cascade action, we would have 0 columns left. Perhaps the wording can be word-smithed to handle both cases well. The error message wording of LANG_CASCADED_GEN_COL_DROP is also confusing to me: "Dropping column ' {0} ' would orphan the generated column ' {1} ' which references it. You must drop the generated column first. I interpret this to mean that if 0 is dropped, 1 would be the only column left and a computed column can obviously not be alone in a table. But the issue would be clearer explained by saying that after 1 is removed by cascade, there would be no columns left. I also don't understand the point about dropping the generated column first? Dropping 0 after having dropped 1 wouldn't improve things unless another column were added in the meantime? Or maybe I am misunderstanding here.. getting late GeneratedColumnsTest.java The verification that an index on a generated column is actually dropped could be better, I think. Presently, you only verify that an insert previosly blocked by the index is legal after the dropping of the column. It would be good to inspect the dictionary to see that it is actually gone physically as well. Under verification that constraints are dropped, I started wondering.. I think this expected error is not according to the standard: : create table t_dc_10( a int generated always as ( -b ) check ( a is not null ), b int, c int )" alter table t_dc_10 drop column a restrict // gives X0Y25 : According to section 11.18, we have: "5) If RESTRICT is specified, then C shall not be referenced in any of the following: a) The <query expression> of any view descriptor. b) The <search condition> of any constraint descriptor other than a table constraint descriptor that contains references to no other column and ********************************************************************* that is included in the table descriptor of T. c) The SQL routine body of any routine descriptor. d) Either an explicit trigger column list or a triggered action column set of any trigger descriptor. e) The generation expression of any column descriptor." It would seem to be that in this case we are covered under item 5b), the check constraint only references the dropped column "a," so RESTRICT should not block this dropping. This is not behavior particular to generated columns, so if you agree with my interpretation, we should file a JIRA for it.
          Hide
          Dag H. Wanvik added a comment -

          Looked at derby-481-13-aa-alterDatatype.diff, looks good! Thanks.

          Show
          Dag H. Wanvik added a comment - Looked at derby-481-13-aa-alterDatatype.diff, looks good! Thanks.
          Hide
          Dag H. Wanvik added a comment -

          Looking at derby-481-12-aa-padding, thanks for the new tests.

          • Shouldn't the type of column a in table t_cc_varchar be a varchar? It is a char now.
          Show
          Dag H. Wanvik added a comment - Looking at derby-481-12-aa-padding, thanks for the new tests. Shouldn't the type of column a in table t_cc_varchar be a varchar? It is a char now.
          Hide
          Dag H. Wanvik added a comment -

          Reviewing derby-481-11-aa-notNull. Thanks for the good explanations,
          Rick, really helped! Good patch.

          Notes:

          Index: java/engine/org/apache/derby/impl/sql/execute/NormalizeResultSet.java

          Good that you refactored this class a bit; was a bit messy before..

          • Javadoc for normalizeColumn starts with the text "Normalize a
            row". It should probably be "Normalize a column". What about the
            rest of that comment? It speaks of not yet having data type
            conversions.. Is this still relevant? We do have casts, but maybe
            this is something else. Asking since you touched it by moving it
            around
          • I think the normalizeColumn method should take sourceCol as an
            argument, rather than sourceRow (less scope; only one column is ever
            used inside normalizeColumn).
          • normalizeColumn javadoc lacks @params and @return.
          • private member numColumns is no longer needed.
          • computeStartColumn lacks @return and @params.

          Index: java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java

          • would be nice to comment usage of new members firstColumn,
            generatedColumnPositions, normalizedGeneratedValues
          • maybe I am missing something here, but couldn't the two conditional
            actions below be put inside one 'if' in that they always happen together?

          if ( firstColumn < 0 )

          { firstColumn = NormalizeResultSet.computeStartColumn( isUpdate, activation.getResultDescription() ); }

          if ( generatedColumnPositions == null )

          { setupGeneratedColumns( activation, (ValueRow) newRow ); }
          • Javadoc for setupGeneratedColumns could use some more explanation, a
            bit on the concise side now, I think. It has side-effects.
          • The javadoc for ResultColumn#hasGenerationClause can now refer to
            the javadoc in the interface ResultColumnDescriptor (to avoid
            redundancy in description).
          Show
          Dag H. Wanvik added a comment - Reviewing derby-481-11-aa-notNull. Thanks for the good explanations, Rick, really helped! Good patch. Notes: Index: java/engine/org/apache/derby/impl/sql/execute/NormalizeResultSet.java Good that you refactored this class a bit; was a bit messy before.. Javadoc for normalizeColumn starts with the text "Normalize a row". It should probably be "Normalize a column". What about the rest of that comment? It speaks of not yet having data type conversions.. Is this still relevant? We do have casts, but maybe this is something else. Asking since you touched it by moving it around I think the normalizeColumn method should take sourceCol as an argument, rather than sourceRow (less scope; only one column is ever used inside normalizeColumn). normalizeColumn javadoc lacks @params and @return. private member numColumns is no longer needed. computeStartColumn lacks @return and @params. Index: java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java would be nice to comment usage of new members firstColumn, generatedColumnPositions, normalizedGeneratedValues maybe I am missing something here, but couldn't the two conditional actions below be put inside one 'if' in that they always happen together? if ( firstColumn < 0 ) { firstColumn = NormalizeResultSet.computeStartColumn( isUpdate, activation.getResultDescription() ); } if ( generatedColumnPositions == null ) { setupGeneratedColumns( activation, (ValueRow) newRow ); } Javadoc for setupGeneratedColumns could use some more explanation, a bit on the concise side now, I think. It has side-effects. The javadoc for ResultColumn#hasGenerationClause can now refer to the javadoc in the interface ResultColumnDescriptor (to avoid redundancy in description).
          Hide
          Dag H. Wanvik added a comment -

          Reviewed patch derby-481-10-aa-foreignKeyActions. Looks correct to
          me. Thanks, Rick!

          My notes:

          • error messages

          '

          {0}

          ' is a generated column. It cannot be part of a foreign key whose
          referential action is SET NULL or SET DEFAULT or whose update rule is
          ON UPDATE CASCADE

          Would perhaps be better to say "referential action for delete is SET
          NULL or SET DEFAULT or whose referential action for update is
          CASCADE" to make the the two cases more symmetrically explained. As
          it stands, the first part doesn't explicitly say this concerns ON
          DELETE.

          • AlterTableNode.java

          This code fragment:
          :
          if (numGenerationClauses > 0)

          { tableElementList.bindAndValidateGenerationClauses(fromList, generatedColumns ); }

          if ( numReferenceConstraints > 0)

          { tableElementList.validateForeignKeysOnGenerationClauses( fromList, generatedColumns ); }

          The latter if statement could be conditional on numGenerationClauses > 0, too,
          I think

          if (numGenerationClauses > 0) {
          tableElementList.bindAndValidateGenerationClauses(
          fromList, generatedColumns );

          if (numReferenceConstraints > 0)

          { tableElementList.validateForeignKeysOnGenerationClauses( fromList, generatedColumns ); }

          }

          Even if validateForeignKeysOnGenerationClauses tests for the presence
          of generation clauses, it seems more logical to me to just not call
          it.

          Ditto in CreateTableNode.java.

          • TableElementList.java
          • typo: "rulse" -> rule
          • This logic
            if (
            ( deleteRule != StatementType.RA_SETNULL ) &&
            ( deleteRule != StatementType.RA_SETDEFAULT )
            ) { continue; }

          I think is more readable as:

          if( ! (deleteRule == StatementType.RA_SETNULL ||
          deleteRule == StatementType.RA_SETDEFAULT ) )

          { continue }

          Ideally, though, I would prefer

          if( deleteRule == StatementType.RA_SETNULL ||
          deleteRule == StatementType.RA_SETDEFAULT )

          { // do our thing }

          else

          { continue }

          in spite of the extra indentation, but that's me...

          • GeneratedColumnsTest.java

          Thanks for the extra tests! Cf. DERBY-3964 though.

          Show
          Dag H. Wanvik added a comment - Reviewed patch derby-481-10-aa-foreignKeyActions. Looks correct to me. Thanks, Rick! My notes: error messages ' {0} ' is a generated column. It cannot be part of a foreign key whose referential action is SET NULL or SET DEFAULT or whose update rule is ON UPDATE CASCADE Would perhaps be better to say "referential action for delete is SET NULL or SET DEFAULT or whose referential action for update is CASCADE" to make the the two cases more symmetrically explained. As it stands, the first part doesn't explicitly say this concerns ON DELETE. AlterTableNode.java This code fragment: : if (numGenerationClauses > 0) { tableElementList.bindAndValidateGenerationClauses(fromList, generatedColumns ); } if ( numReferenceConstraints > 0) { tableElementList.validateForeignKeysOnGenerationClauses( fromList, generatedColumns ); } The latter if statement could be conditional on numGenerationClauses > 0, too, I think if (numGenerationClauses > 0) { tableElementList.bindAndValidateGenerationClauses( fromList, generatedColumns ); if (numReferenceConstraints > 0) { tableElementList.validateForeignKeysOnGenerationClauses( fromList, generatedColumns ); } } Even if validateForeignKeysOnGenerationClauses tests for the presence of generation clauses, it seems more logical to me to just not call it. Ditto in CreateTableNode.java. TableElementList.java typo: "rulse" -> rule This logic if ( ( deleteRule != StatementType.RA_SETNULL ) && ( deleteRule != StatementType.RA_SETDEFAULT ) ) { continue; } I think is more readable as: if( ! (deleteRule == StatementType.RA_SETNULL || deleteRule == StatementType.RA_SETDEFAULT ) ) { continue } Ideally, though, I would prefer if( deleteRule == StatementType.RA_SETNULL || deleteRule == StatementType.RA_SETDEFAULT ) { // do our thing } else { continue } in spite of the extra indentation, but that's me... GeneratedColumnsTest.java Thanks for the extra tests! Cf. DERBY-3964 though.
          Hide
          Dag H. Wanvik added a comment -

          Reviewed derby-481-09-aa-dummyDefaults.
          Patch looks fine.

          • InsertNode.java
            Comment ca line 301 "Replace any DEFAULTs with the associated tree"
            is now only conditionally true; for generated columns
            this happens later. It would be nice to improve this comment to
            explain where it happens for such column.

          The javadoc for the ResultSetNode#replaceDefaults would also benefit from this caveat.
          Similarly for RowResultSet, SingleChildResultSetNode and TableOperatorNode, I guess..

          Show
          Dag H. Wanvik added a comment - Reviewed derby-481-09-aa-dummyDefaults. Patch looks fine. InsertNode.java Comment ca line 301 "Replace any DEFAULTs with the associated tree" is now only conditionally true; for generated columns this happens later. It would be nice to improve this comment to explain where it happens for such column. The javadoc for the ResultSetNode#replaceDefaults would also benefit from this caveat. Similarly for RowResultSet, SingleChildResultSetNode and TableOperatorNode, I guess..
          Hide
          Dag H. Wanvik added a comment -

          Thanks for your incorporating the comments, Rick!

          Reviewed derby-481-08-aa-castToDeclaredType.diff

          Patch looks good!

          Some small notes on the test: I compared with the
          table found in the refman under the CAST function
          (http://db.apache.org/derby/docs/dev/ref/rrefsqlj33562.html) to check
          that all conversions are included.

          • smallint, int, bigint and decimal lacks a testcase for char
          • char lacks testcases for smallint, int, bigint and decimal
          • the varchar test seems wrong (identical to the char case; copy/paste
            error?)
          • the three bitdata cases lack cases for clob and blob
          • timestamp lacks cases for date and time
          • xml is omitted

          Some of the omissions may be intentional (xml), if so it would be good
          to indicate why they are omitted in this test.

          Show
          Dag H. Wanvik added a comment - Thanks for your incorporating the comments, Rick! Reviewed derby-481-08-aa-castToDeclaredType.diff Patch looks good! Some small notes on the test: I compared with the table found in the refman under the CAST function ( http://db.apache.org/derby/docs/dev/ref/rrefsqlj33562.html ) to check that all conversions are included. smallint, int, bigint and decimal lacks a testcase for char char lacks testcases for smallint, int, bigint and decimal the varchar test seems wrong (identical to the char case; copy/paste error?) the three bitdata cases lack cases for clob and blob timestamp lacks cases for date and time xml is omitted Some of the omissions may be intentional (xml), if so it would be good to indicate why they are omitted in this test.
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me except for the heisenbugs in the stressmulti tests. Committed derby-481-19-aa-cleanup.diff at subversion revision 719760.

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me except for the heisenbugs in the stressmulti tests. Committed derby-481-19-aa-cleanup.diff at subversion revision 719760.
          Rick Hillegas made changes -
          Attachment derby-481-19-aa-cleanup.diff [ 12394442 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-19-aa-cleanup.diff, a cleanup patch incorporating feedback from Dag and Knut on previous patches. Thanks, Dag and Knut! Running tests now. Touches the following files:

          M java/engine/org/apache/derby/impl/sql/compile/DDLStatementNode.java
          M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java
          M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java
          M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java
          M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java
          M java/engine/org/apache/derby/impl/sql/compile/GenerationClauseNode.java
          M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java
          M java/engine/org/apache/derby/iapi/util/StringUtil.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsHelper.java

          Show
          Rick Hillegas added a comment - Attaching derby-481-19-aa-cleanup.diff, a cleanup patch incorporating feedback from Dag and Knut on previous patches. Thanks, Dag and Knut! Running tests now. Touches the following files: M java/engine/org/apache/derby/impl/sql/compile/DDLStatementNode.java M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java M java/engine/org/apache/derby/impl/sql/compile/GenerationClauseNode.java M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java M java/engine/org/apache/derby/iapi/util/StringUtil.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsHelper.java
          Rick Hillegas made changes -
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-18-aa-updatableResultSets.diff. This patch adds some tests to verify that you can't use updatable ResultSets to corrupt generated columns. Touches the following files:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsHelper.java

          Committed at subversion revision 719656.

          Show
          Rick Hillegas added a comment - Attaching derby-481-18-aa-updatableResultSets.diff. This patch adds some tests to verify that you can't use updatable ResultSets to corrupt generated columns. Touches the following files: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsHelper.java Committed at subversion revision 719656.
          Rick Hillegas made changes -
          Link This issue relates to DERBY-3959 [ DERBY-3959 ]
          Kim Haase made changes -
          Link This issue blocks DERBY-3959 [ DERBY-3959 ]
          Rick Hillegas made changes -
          Attachment GeneratedColumns.html [ 12394424 ]
          Hide
          Rick Hillegas added a comment -

          Attaching a new version of the functional spec. This repairs the description of ALTER COLUMN in the Behavior section. That bullet had been cut off.

          Show
          Rick Hillegas added a comment - Attaching a new version of the functional spec. This repairs the description of ALTER COLUMN in the Behavior section. That bullet had been cut off.
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3957 [ DERBY-3957 ]
          Hide
          Dag H. Wanvik added a comment -

          Reviewed derby-481-07-ab-noSQLinRoutines. Good patch, verified that
          the extended test fails without the rest of the patch. Notes:

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

          • throwReliabilityException: new argument fragmentBitMask lacks @param

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

          • You have used the reliability constant SQL_IN_ROUTINES_ILLEGAL.
            Isn't the name "routines" that overly wide here, since only functions are
            ever allowed in generation clauses?
          Show
          Dag H. Wanvik added a comment - Reviewed derby-481-07-ab-noSQLinRoutines. Good patch, verified that the extended test fails without the rest of the patch. Notes: M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java throwReliabilityException: new argument fragmentBitMask lacks @param M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java You have used the reliability constant SQL_IN_ROUTINES_ILLEGAL. Isn't the name "routines" that overly wide here, since only functions are ever allowed in generation clauses?
          Hide
          Dag H. Wanvik added a comment -

          Reviewed derby-481-06-aa-genreferences. Looks good! I verified that without the rest of this
          patch, the extended GeneratedColumnsTest fails on the first case of
          referencing other generated columns. Notes:

          • The javadoc for TableDescriptor#makeColumnMap lacks @param and @return tags.
          • The javadoc for ResultColumnList#getPosition has @param, but lacks @return tag.
          Show
          Dag H. Wanvik added a comment - Reviewed derby-481-06-aa-genreferences. Looks good! I verified that without the rest of this patch, the extended GeneratedColumnsTest fails on the first case of referencing other generated columns. Notes: The javadoc for TableDescriptor#makeColumnMap lacks @param and @return tags. The javadoc for ResultColumnList#getPosition has @param, but lacks @return tag.
          Hide
          Dag H. Wanvik added a comment -

          Reviewed derby-481-05-aa-update. Thanks for all this work, Rick!
          Thorough work, I think, and the approach is well integrated with the rest of the
          machinery. Explanations again helped me through this one; some
          unclear points ("why is this needed?") from the the preceding "insert" patch became
          clear now.

          Minor notes:

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

          • import of C_NodeTypes occurs twice now. New imports of
            java.util.ArrayList and java.util.Array is also unneeded.
          • comment ca line 578 starting "Get and bind all check constraints.."
            should mention generated columns too.
          • addGeneratedColumns's dummy declaration can omitted by using
            getNullNode(gc.getType()) instead.
          • At declaration time of bindStatement's addedGeneratedColumns, it
            would be nice to mention what this variable is used for (grant
            "forgiveness" in forbidGenerationOverrides).

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

          • pedantic note: assertTriggerStatus makes a select which does no
            ORDER BY before comparison with canon; it's OK here of course, since
            it's a VTI for which we have control over the order of the rows
            handed out..
          Show
          Dag H. Wanvik added a comment - Reviewed derby-481-05-aa-update. Thanks for all this work, Rick! Thorough work, I think, and the approach is well integrated with the rest of the machinery. Explanations again helped me through this one; some unclear points ("why is this needed?") from the the preceding "insert" patch became clear now. Minor notes: M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java import of C_NodeTypes occurs twice now. New imports of java.util.ArrayList and java.util.Array is also unneeded. comment ca line 578 starting "Get and bind all check constraints.." should mention generated columns too. addGeneratedColumns's dummy declaration can omitted by using getNullNode(gc.getType()) instead. At declaration time of bindStatement's addedGeneratedColumns, it would be nice to mention what this variable is used for (grant "forgiveness" in forbidGenerationOverrides). M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java pedantic note: assertTriggerStatus makes a select which does no ORDER BY before comparison with canon; it's OK here of course, since it's a VTI for which we have control over the order of the rows handed out..
          Hide
          Dag H. Wanvik added a comment -

          Reviewed derby-481-04-aa-insert. Quite a mouthful, this one Looks
          good, though. Thanks for the detailed explanation of the code; very
          helpful. I wasn't able to locate the "similar poking" being done for
          the CHECK codepaths; could you point me to that?

          Minor notes:

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

          • I think createGeneratedColumn can be simplified by using arg
            getNullNode(colDesc.getType()) instead of the dummy declaration?

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

          • javadocs for forbidGenerationOverrides, parseAndBindGenerationClauses
            and generateGenerationClauses lack @params
          • The "continue" statement after hasGenerationClauses has been
            established as true can be replaced by a break; no need to see more.
            The initial comment in this loop ("generate statements of the
            form..") seem misplaced too, since this loop is only used to detect
            is there exists a generation clause. It should probably be moved to
            after the loop?
          • I see you have just followed the pattern from check constraints, but
            I am not sure I like the fact that there are two overloaded public
            methods called generateGenerationClauses which seem to be on
            different levels of abstraction. The innermost can be private,
            btw. I would have preferred different names for them.

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

          • has lots of spurious diffs; only one significant change.

          M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java

          • The method evaluateGenerationClauses could be made protected and
            moved down to DMLWriteResultSet. It lacks @params for its javadoc.
          Show
          Dag H. Wanvik added a comment - Reviewed derby-481-04-aa-insert. Quite a mouthful, this one Looks good, though. Thanks for the detailed explanation of the code; very helpful. I wasn't able to locate the "similar poking" being done for the CHECK codepaths; could you point me to that? Minor notes: M java/engine/org/apache/derby/impl/sql/compile/ResultSetNode.java I think createGeneratedColumn can be simplified by using arg getNullNode(colDesc.getType()) instead of the dummy declaration? M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java javadocs for forbidGenerationOverrides, parseAndBindGenerationClauses and generateGenerationClauses lack @params The "continue" statement after hasGenerationClauses has been established as true can be replaced by a break; no need to see more. The initial comment in this loop ("generate statements of the form..") seem misplaced too, since this loop is only used to detect is there exists a generation clause. It should probably be moved to after the loop? I see you have just followed the pattern from check constraints, but I am not sure I like the fact that there are two overloaded public methods called generateGenerationClauses which seem to be on different levels of abstraction. The innermost can be private, btw. I would have preferred different names for them. M java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java has lots of spurious diffs; only one significant change. M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java The method evaluateGenerationClauses could be made protected and moved down to DMLWriteResultSet. It lacks @params for its javadoc.
          Hide
          Dag H. Wanvik added a comment -

          > I think that if views are supposed to be dropped because an
          > underlying, role-based permission has vanished, then tables should
          > also be dropped if they require a role-based permission that has
          > vanished.

          I am not sure; I haven't yet been able to find any mention of this
          scneario in the standard. It seems a bit harsh; dropping a view,
          constraint or trigger doesn't actually delete any data, whereas
          deleting a table would..

          Maybe it's ok to just let inserts and updates (if the generated column
          would need regeneration) to that table fail as long as execute
          privileges are not in place for any function referenced in the
          generation expression.. I'll see if I can find anything in the std.

          Show
          Dag H. Wanvik added a comment - > I think that if views are supposed to be dropped because an > underlying, role-based permission has vanished, then tables should > also be dropped if they require a role-based permission that has > vanished. I am not sure; I haven't yet been able to find any mention of this scneario in the standard. It seems a bit harsh; dropping a view, constraint or trigger doesn't actually delete any data, whereas deleting a table would.. Maybe it's ok to just let inserts and updates (if the generated column would need regeneration) to that table fail as long as execute privileges are not in place for any function referenced in the generation expression.. I'll see if I can find anything in the std.
          Hide
          Dag H. Wanvik added a comment -

          Reviewed derby-481-03-aa-grammar.diff, looks good! Interesting to see more of the compile-time
          mechinery involved here.

          Minor notes:

          • DDLStatementNode.java
            makeFromList javadoc misses @param for 2 parameters
          • TableElementList.java
          • This comment in bindAndValidateGenerationClauses is a cut/paste palimpsest:
            // bind the check condition
            // verify that it evaluates to a boolean
          • Shouldn't finally clause in bindAndValidateGenerationClauses also
            reset the current auxiliary provider list?
          • StaticMethodCallNode.java
          • two spurious blank line diffs
          • GenerationClauseNode.java
            toString should include "AS" keyword after ALWAYS.
          • GeneratedColumnsTest.java
          • It would be nice to add a comment that the test can not be run
            in client server mode due to the casting to internal Derby objects
            of some result set columns for dictionary tables.
          • DB_Table.java

          Will an unqualified function name necessarily be output correctly by
          dblook? Do we need to expand the generation text in SYSCOLUMN to
          include the fully qualified name for functions?

          Show
          Dag H. Wanvik added a comment - Reviewed derby-481-03-aa-grammar.diff, looks good! Interesting to see more of the compile-time mechinery involved here. Minor notes: DDLStatementNode.java makeFromList javadoc misses @param for 2 parameters TableElementList.java This comment in bindAndValidateGenerationClauses is a cut/paste palimpsest: // bind the check condition // verify that it evaluates to a boolean Shouldn't finally clause in bindAndValidateGenerationClauses also reset the current auxiliary provider list? StaticMethodCallNode.java two spurious blank line diffs GenerationClauseNode.java toString should include "AS" keyword after ALWAYS. GeneratedColumnsTest.java It would be nice to add a comment that the test can not be run in client server mode due to the casting to internal Derby objects of some result set columns for dictionary tables. DB_Table.java Will an unqualified function name necessarily be output correctly by dblook? Do we need to expand the generation text in SYSCOLUMN to include the fully qualified name for functions?
          Rick Hillegas made changes -
          Attachment derby-481-17-aa-currentSchema.diff [ 12393971 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-17-aa-currentSchema.diff which adds test to GeneratedColumnsTest to verify that generation clauses can't mention unstable system functions CURRENT SCHEMA and CURRENT SQLID. Committed at subversion revision 714188.

          Show
          Rick Hillegas added a comment - Attaching derby-481-17-aa-currentSchema.diff which adds test to GeneratedColumnsTest to verify that generation clauses can't mention unstable system functions CURRENT SCHEMA and CURRENT SQLID. Committed at subversion revision 714188.
          Rick Hillegas made changes -
          Attachment GeneratedColumns.html [ 12393960 ]
          Hide
          Rick Hillegas added a comment -

          Attaching a revised version of the functional spec, addressing Dag's comments. Thanks, Dag!

          Show
          Rick Hillegas added a comment - Attaching a revised version of the functional spec, addressing Dag's comments. Thanks, Dag!
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3950 [ DERBY-3950 ]
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3949 [ DERBY-3949 ]
          Hide
          Rick Hillegas added a comment -

          Hi Dag,

          Thanks for your continued attention to this issue. I am catching up with some of your comments now. This is reference to the role-related question you asked on 12/Nov/08 09:49 AM. I think that if views are supposed to be dropped because an underlying, role-based permission has vanished, then tables should also be dropped if they require a role-based permission that has vanished. I am going to create an issue to track this. If you think this is not the correct behavior, please elaborate. Thanks.

          Show
          Rick Hillegas added a comment - Hi Dag, Thanks for your continued attention to this issue. I am catching up with some of your comments now. This is reference to the role-related question you asked on 12/Nov/08 09:49 AM. I think that if views are supposed to be dropped because an underlying, role-based permission has vanished, then tables should also be dropped if they require a role-based permission that has vanished. I am going to create an issue to track this. If you think this is not the correct behavior, please elaborate. Thanks.
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3948 [ DERBY-3948 ]
          Hide
          Knut Anders Hatlen added a comment -

          All callers of StringUtil.stringify() were removed in derby-481-14-ab-dropColumn.diff, so we might just as well remove the method now.

          Show
          Knut Anders Hatlen added a comment - All callers of StringUtil.stringify() were removed in derby-481-14-ab-dropColumn.diff, so we might just as well remove the method now.
          Hide
          Knut Anders Hatlen added a comment -

          > - javadoc for StringUtil#stringify: would be nice to specify the format of
          > the output: Some such:
          >
          > 'null' | '[ ' [ <decimal int>

          { ', ' <decimal int> }

          * ] ' ]'

          Except for the space after the opening bracket and before the closing
          bracket, the format is identical to what you get from
          java.util.Arrays.toString(int[]), which is not available prior to Java
          1.5. Perhaps it's better just to document that it should return the
          same as Arrays.toString() so that we can replace it with a call to the
          standard library once we move to Java 1.5. Currently, the the method
          is only used by test code, so maybe it should be moved out of the
          production jars?

          > - GeneratedColumnsTest#expectExecutionError: closing of ps is missing

          Or even better: use BaseJDBCTestCase.prepareStatement() instead of
          Connection.prepareStatement(). Then you get automatic cleanup in
          tearDown().

          Show
          Knut Anders Hatlen added a comment - > - javadoc for StringUtil#stringify: would be nice to specify the format of > the output: Some such: > > 'null' | '[ ' [ <decimal int> { ', ' <decimal int> } * ] ' ]' Except for the space after the opening bracket and before the closing bracket, the format is identical to what you get from java.util.Arrays.toString(int[]), which is not available prior to Java 1.5. Perhaps it's better just to document that it should return the same as Arrays.toString() so that we can replace it with a call to the standard library once we move to Java 1.5. Currently, the the method is only used by test code, so maybe it should be moved out of the production jars? > - GeneratedColumnsTest#expectExecutionError: closing of ps is missing Or even better: use BaseJDBCTestCase.prepareStatement() instead of Connection.prepareStatement(). Then you get automatic cleanup in tearDown().
          Hide
          Dag H. Wanvik added a comment -

          Reviewing patch derby-481-02-aa-utilities.diff.

          Patch looks good to me. Some small notes:

          • javadoc for StringUtil#stringify: would be nice to specify the format of
            the output: Some such:

          'null' | '[ ' [ <decimal int>

          { ', ' <decimal int> }

          * ] ' ]'

          • GeneratedColumnsTest#expectExecutionError: closing of ps is missing
          • GeneratedColumnsTest#assertResults
            :
            assertEquals( (expectedValue == null), rs.wasNull() );

          if ( expectedValue == null )

          { assertNull( actualValue ); }

          Isn't the latter assert redundant after the first assert?

          • GeneratedColumnsTest: lines > 80 long
          • SQLState.java: it would look better to align the values for the new
            errors with the existing declarations (in column 72).

          I didn't check the commented out new code much, since its not live yet.

          Show
          Dag H. Wanvik added a comment - Reviewing patch derby-481-02-aa-utilities.diff. Patch looks good to me. Some small notes: javadoc for StringUtil#stringify: would be nice to specify the format of the output: Some such: 'null' | '[ ' [ <decimal int> { ', ' <decimal int> } * ] ' ]' GeneratedColumnsTest#expectExecutionError: closing of ps is missing GeneratedColumnsTest#assertResults : assertEquals( (expectedValue == null), rs.wasNull() ); if ( expectedValue == null ) { assertNull( actualValue ); } Isn't the latter assert redundant after the first assert? GeneratedColumnsTest: lines > 80 long SQLState.java: it would look better to align the values for the new errors with the existing declarations (in column 72). I didn't check the commented out new code much, since its not live yet.
          Hide
          Dag H. Wanvik added a comment -

          I read the specification again, clear and easy to read!

          Some notes:

          • Overview
            2nd sentence. Maybe add ALTER TABLE as well.
            3rd sentence. "The user declares" -> "The user can declare"
          • SQL standard:
            Add sections 11.8, 11.39
            Reference to section 14.18 should be to 11.18, I think
          • syntax:
            generation-clause ::= GENERATED ALWAYS ( value-expression )
                                                                                  • "AS" keyword required after ALWAYS but missing
          • SYSCOLUMNS:
            no new column was added, so I guess this section should be modified to
            reflect the implementation now?
          • Documentation

          If we decide REVOKE execute privilege from <role> will impact functions
          references in generated columns (as in dropping them), this should probably be documented
          along with the new doc for REVOKE <priv> from <role> and DROP
          <role>.

          Add note in INSERT on the use of "default", perhaps?

          Appendix A: Possibly Non-deterministic System Functions

          • add CURRENT SCHEMA
          • add CURRENT SQLID
          Show
          Dag H. Wanvik added a comment - I read the specification again, clear and easy to read! Some notes: Overview 2nd sentence. Maybe add ALTER TABLE as well. 3rd sentence. "The user declares" -> "The user can declare" SQL standard: Add sections 11.8, 11.39 Reference to section 14.18 should be to 11.18, I think syntax: generation-clause ::= GENERATED ALWAYS ( value-expression ) "AS" keyword required after ALWAYS but missing SYSCOLUMNS: no new column was added, so I guess this section should be modified to reflect the implementation now? Documentation If we decide REVOKE execute privilege from <role> will impact functions references in generated columns (as in dropping them), this should probably be documented along with the new doc for REVOKE <priv> from <role> and DROP <role>. Add note in INSERT on the use of "default", perhaps? Appendix A: Possibly Non-deterministic System Functions add CURRENT SCHEMA add CURRENT SQLID
          Hide
          Rick Hillegas added a comment -

          Thanks for your comments, Dag and Knut. Knut is right, the following statements should fail because we are trying to override a generation clause:

          insert into a select * from b;
          insert into a select i,0 from b;

          Thanks for finding these problems.

          Show
          Rick Hillegas added a comment - Thanks for your comments, Dag and Knut. Knut is right, the following statements should fail because we are trying to override a generation clause: insert into a select * from b; insert into a select i,0 from b; Thanks for finding these problems.
          Hide
          Dag H. Wanvik added a comment -

          Knut> I would have expected that attempts to set a column which is
          Knut> GENERATED ALWAYS AS failed.

          Yes. Note, though, that the generated column gets the correct
          (generated) value, Derby presently just requires the number of columns
          to be 2, the last (2nd) column in the RS is discarded...

          Knut> insert into a select i from b

          Yes, I was just wondering if the column specification could be dropped
          legally in this case or not. If yes, the required result set should
          have 1, not 2 columns. But it might be column indication is always
          required here.

          Show
          Dag H. Wanvik added a comment - Knut> I would have expected that attempts to set a column which is Knut> GENERATED ALWAYS AS failed. Yes. Note, though, that the generated column gets the correct (generated) value, Derby presently just requires the number of columns to be 2, the last (2nd) column in the RS is discarded... Knut> insert into a select i from b Yes, I was just wondering if the column specification could be dropped legally in this case or not. If yes, the required result set should have 1, not 2 columns. But it might be column indication is always required here.
          Hide
          Knut Anders Hatlen added a comment -

          > Is this the right behavior?

          I would have expected that attempts to set a column which is GENERATED
          ALWAYS AS failed. Similarly to what happens if you try to specify the
          value of an identity column (GENERATED ALWAYS AS IDENTITY):

          ij> insert into tt select * from b;
          ERROR 42Z23: Attempt to modify an identity column 'B'.

          > It seems a bit non-intuitive to require the
          > result set inserted to have two columns when only one is used anyway...

          Note that you could specify that you only set some of the columns in
          the insert (again similarly to how you'd do it if you had an identity
          column):

          insert into a select i from b

          Show
          Knut Anders Hatlen added a comment - > Is this the right behavior? I would have expected that attempts to set a column which is GENERATED ALWAYS AS failed. Similarly to what happens if you try to specify the value of an identity column (GENERATED ALWAYS AS IDENTITY): ij> insert into tt select * from b; ERROR 42Z23: Attempt to modify an identity column 'B'. > It seems a bit non-intuitive to require the > result set inserted to have two columns when only one is used anyway... Note that you could specify that you only set some of the columns in the insert (again similarly to how you'd do it if you had an identity column): insert into a select i from b
          Hide
          Dag H. Wanvik added a comment -

          Looked at derby-481-01-aa-catalog.diff, looks good. Only
          small notes:

          • weird indentation for variable "count" in getGeneratedColumns
          • In DefaultInfoImpl, specify BITS_MASK_IS_GENERATED_COLUMN as = 0x1 << 1 to follow
            pattern ?
          • New DefaultInfoImpl constructor has weird indentation for
            referencedColumnIDs arg.
          • Lines > 80 chars
          Show
          Dag H. Wanvik added a comment - Looked at derby-481-01-aa-catalog.diff, looks good. Only small notes: weird indentation for variable "count" in getGeneratedColumns In DefaultInfoImpl, specify BITS_MASK_IS_GENERATED_COLUMN as = 0x1 << 1 to follow pattern ? New DefaultInfoImpl constructor has weird indentation for referencedColumnIDs arg. Lines > 80 chars
          Hide
          Dag H. Wanvik added a comment -

          Is this the right behavior? It seems a bit non-intuitive to require the
          result set inserted to have two columns when only one is used anyway...

          ij> create table a(i int, j int generated always as (-i));
          0 rows inserted/updated/deleted
          ij> create table b(i int, j int);
          0 rows inserted/updated/deleted
          ij> insert into b values (1,1);
          1 row inserted/updated/deleted
          ij> insert into a select * from b;
          1 row inserted/updated/deleted
          ij> select * from a;
          I |J
          -----------------------
          1 |-1

          1 row selected
          ij> insert into a select i from b;
          ERROR 42802: The number of values assigned is not the same as the number of specified or implied columns.
          ij> insert into a select i,0 from b;
          1 row inserted/updated/deleted
          ij> select * from a;
          I |J
          -----------------------
          1 |-1
          1 |-1

          2 rows selected

          Show
          Dag H. Wanvik added a comment - Is this the right behavior? It seems a bit non-intuitive to require the result set inserted to have two columns when only one is used anyway... ij> create table a(i int, j int generated always as (-i)); 0 rows inserted/updated/deleted ij> create table b(i int, j int); 0 rows inserted/updated/deleted ij> insert into b values (1,1); 1 row inserted/updated/deleted ij> insert into a select * from b; 1 row inserted/updated/deleted ij> select * from a; I |J ----------------------- 1 |-1 1 row selected ij> insert into a select i from b; ERROR 42802: The number of values assigned is not the same as the number of specified or implied columns. ij> insert into a select i,0 from b; 1 row inserted/updated/deleted ij> select * from a; I |J ----------------------- 1 |-1 1 |-1 2 rows selected
          Hide
          Dag H. Wanvik added a comment -

          I agree.

          Another thing:

          What is the correct behavior for this case? Here the generation
          expression needs a privilege which gets revoked...

          ij(CONNECTION0)> create table t (i int, j int, k int generated always as (dag.myfunc(j)));
          ERROR 42504: User 'KNUT' does not have execute permission on FUNCTION 'DAG'.'MYFUNC'.
          ij(CONNECTION0)> set role bar; – this role has the privilege and is granted to KNUT earlier
          0 rows inserted/updated/deleted
          ij(CONNECTION0)> create table t (i int, j int, k int generated always as (dag.myfunc(j)));
          0 rows inserted/updated/deleted
          ij(CONNECTION0)> set connection connection1;
          ij(CONNECTION1)> revoke bar from knut;
          0 rows inserted/updated/deleted
          ij(CONNECTION1)> set connection connection0;
          ij(CONNECTION0)> insert into t values (1,2,default);
          ERROR 42504: User 'KNUT' does not have execute permission on FUNCTION 'DAG'.'MYFUNC'.

          Triggers, view and constraints in similar circumstances get dropped; should the generated column get dropped too?

          Show
          Dag H. Wanvik added a comment - I agree. Another thing: What is the correct behavior for this case? Here the generation expression needs a privilege which gets revoked... ij(CONNECTION0)> create table t (i int, j int, k int generated always as (dag.myfunc(j))); ERROR 42504: User 'KNUT' does not have execute permission on FUNCTION 'DAG'.'MYFUNC'. ij(CONNECTION0)> set role bar; – this role has the privilege and is granted to KNUT earlier 0 rows inserted/updated/deleted ij(CONNECTION0)> create table t (i int, j int, k int generated always as (dag.myfunc(j))); 0 rows inserted/updated/deleted ij(CONNECTION0)> set connection connection1; ij(CONNECTION1)> revoke bar from knut; 0 rows inserted/updated/deleted ij(CONNECTION1)> set connection connection0; ij(CONNECTION0)> insert into t values (1,2,default); ERROR 42504: User 'KNUT' does not have execute permission on FUNCTION 'DAG'.'MYFUNC'. Triggers, view and constraints in similar circumstances get dropped; should the generated column get dropped too?
          Hide
          Rick Hillegas added a comment -

          Thanks for finding this issue with BEFORE triggers, Dag. I don't understand why the standard forbids these references in BEFORE triggers. This would be a good question to ask the SQL committee. I think we should follow the standard here. We can relax this restriction later if we come to understand that it really is harmless.

          Show
          Rick Hillegas added a comment - Thanks for finding this issue with BEFORE triggers, Dag. I don't understand why the standard forbids these references in BEFORE triggers. This would be a good question to ask the SQL committee. I think we should follow the standard here. We can relax this restriction later if we come to understand that it really is harmless.
          Hide
          Dag H. Wanvik added a comment -

          I am trying to understand if the following is allowed by the standard:
          Assume this ij fragment:

          > create table t (i int, j int, k int generated always as (-j));
          > create function myfunc (i int) returns int language java external name
          > 'M.myfunc' parameter style java;
          >
          > insert into t values (1,2,default), (3,4,default);
          > create trigger tr no cascade before update on t
          > referencing new as new for each row values myfunc(new.k);
          >
          > update t set j=10 where i=1;
          >
          > // myfunc prints -10 here

          In the standard, section 11.39, SR 12 c) I see this provision:

          12) If BEFORE is specified, then:
          :
          c) The <triggered action> shall not contain a <field reference> that
          references a field in the new transition variable corresponding to a
          generated column of T.

          It would seem that the reference to k is illegal here. What do you
          think? Not sure I understand why this should be prohibited, though....

          Show
          Dag H. Wanvik added a comment - I am trying to understand if the following is allowed by the standard: Assume this ij fragment: > create table t (i int, j int, k int generated always as (-j)); > create function myfunc (i int) returns int language java external name > 'M.myfunc' parameter style java; > > insert into t values (1,2,default), (3,4,default); > create trigger tr no cascade before update on t > referencing new as new for each row values myfunc(new.k); > > update t set j=10 where i=1; > > // myfunc prints -10 here In the standard, section 11.39, SR 12 c) I see this provision: 12) If BEFORE is specified, then: : c) The <triggered action> shall not contain a <field reference> that references a field in the new transition variable corresponding to a generated column of T. It would seem that the reference to k is illegal here. What do you think? Not sure I understand why this should be prohibited, though....
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3945 [ DERBY-3945 ]
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me on derby-481-16-aa-dropFunction.diff. Committed at subversion revision 712840.

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me on derby-481-16-aa-dropFunction.diff. Committed at subversion revision 712840.
          Rick Hillegas made changes -
          Attachment derby-481-16-aa-dropFunction.diff [ 12393653 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-16-aa-dropFunction.diff. Running tests now. This patch prevents you from dropping a function mentioned in a generation clause, per the spec.

          The patch builds on the fact that generation clauses are implemented as a kind of default. Defaults, in turn, are a persistent object for which we already have good support in our dependency subsystem.

          Touches the following files:

          M java/engine/org/apache/derby/impl/sql/execute/ColumnInfo.java

          Adds dependency information to this driving data structure.

          M java/engine/org/apache/derby/impl/sql/execute/CreateTableConstantAction.java
          M java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java
          M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java

          Adds bind-time logic to fill the ColumnInfo with the dependencies of generated columns. These dependencies were already being identified but the information was being thrown on the floor.

          M java/engine/org/apache/derby/impl/sql/execute/DDLConstantAction.java
          M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java
          M java/engine/org/apache/derby/impl/sql/compile/CreateViewNode.java

          Adds execution-time logic to hammer the dependencies into SYS.SYSDEPENDS. The enforcement of the dependencies is handled by existing machinery in the dependency subsystem.

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

          Adds test cases.

          Show
          Rick Hillegas added a comment - Attaching derby-481-16-aa-dropFunction.diff. Running tests now. This patch prevents you from dropping a function mentioned in a generation clause, per the spec. The patch builds on the fact that generation clauses are implemented as a kind of default. Defaults, in turn, are a persistent object for which we already have good support in our dependency subsystem. Touches the following files: M java/engine/org/apache/derby/impl/sql/execute/ColumnInfo.java Adds dependency information to this driving data structure. M java/engine/org/apache/derby/impl/sql/execute/CreateTableConstantAction.java M java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java Adds bind-time logic to fill the ColumnInfo with the dependencies of generated columns. These dependencies were already being identified but the information was being thrown on the floor. M java/engine/org/apache/derby/impl/sql/execute/DDLConstantAction.java M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java M java/engine/org/apache/derby/impl/sql/compile/CreateViewNode.java Adds execution-time logic to hammer the dependencies into SYS.SYSDEPENDS. The enforcement of the dependencies is handled by existing machinery in the dependency subsystem. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Adds test cases.
          Rick Hillegas made changes -
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-15-aa-renameAndAddDefault.diff. Tests ran cleanly for me. Committed at subversion revision 712664.

          This patch makes two changes, per the spec:

          A) Prevents users from renaming columns which are referenced by generation clauses.

          B) Prevents users from using ALTER TABLE to add defaults to generated columns. Note that the parser already prevents you from adding defaults to generated columns via CREATE TABLE.

          Touches the following files:

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

          Raises an error if you try to rename a column referenced by a generation clause.

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

          Raises an error if you try to add a default to a generated column.

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

          New error messages for the newly forbidden conditions.

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

          New test cases.

          Show
          Rick Hillegas added a comment - Attaching derby-481-15-aa-renameAndAddDefault.diff. Tests ran cleanly for me. Committed at subversion revision 712664. This patch makes two changes, per the spec: A) Prevents users from renaming columns which are referenced by generation clauses. B) Prevents users from using ALTER TABLE to add defaults to generated columns. Note that the parser already prevents you from adding defaults to generated columns via CREATE TABLE. Touches the following files: M java/engine/org/apache/derby/impl/sql/compile/RenameNode.java Raises an error if you try to rename a column referenced by a generation clause. M java/engine/org/apache/derby/impl/sql/compile/ModifyColumnNode.java Raises an error if you try to add a default to a generated column. M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java New error messages for the newly forbidden conditions. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java New test cases.
          Hide
          Rick Hillegas added a comment -

          Committed derby-481-14-ab-dropColumn.diff at subversion revision 712400.

          Show
          Rick Hillegas added a comment - Committed derby-481-14-ab-dropColumn.diff at subversion revision 712400.
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me on derby-481-14-ab-dropColumn.diff except for 4 errors. These errors were in ClobUpdatableReaderTest, NetworkServerMBeanTest, and StressMultiTest. I think these are either errors introduced by recent submissions or outstanding heisenbugs:

          There were 4 errors:
          1) testUpdateableStoreReader(org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest)java.sql.SQLException: Java exception: 'Bad file descriptor: java.io.IOException'.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45)
          at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:87)
          at org.apache.derby.impl.jdbc.Util.javaException(Util.java:244)
          at org.apache.derby.impl.jdbc.EmbedConnection.clearLOBMapping(EmbedConnection.java:3097)
          at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1765)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.rollback(BaseJDBCTestCase.java:378)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest.tearDown(ClobUpdatableReaderTest.java:304)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          Caused by: java.io.IOException: Bad file descriptor
          at java.io.RandomAccessFile.close0(Native Method)
          at java.io.RandomAccessFile.close(RandomAccessFile.java:532)
          at org.apache.derby.impl.jdbc.LOBFile.close(LOBFile.java:148)
          at org.apache.derby.impl.jdbc.EmbedConnection.clearLOBMapping(EmbedConnection.java:3095)
          ... 29 more
          2) testUpdateableReader(org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest)java.sql.SQLException: Table/View 'UPDATECLOB' already exists in Schema 'APP'.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45)
          at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:201)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:391)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346)
          at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2201)
          at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:81)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1323)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest.setUp(ClobUpdatableReaderTest.java:284)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          Caused by: ERROR X0Y32: Table/View 'UPDATECLOB' already exists in Schema 'APP'.
          at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:373)
          at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.duplicateDescriptorException(DataDictionaryImpl.java:1817)
          at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.addDescriptor(DataDictionaryImpl.java:1808)
          at org.apache.derby.impl.sql.execute.CreateTableConstantAction.executeConstantAction(CreateTableConstantAction.java:238)
          at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:416)
          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:297)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1235)
          ... 29 more
          3) testMultiplexedOperationProblem(org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest)java.sql.SQLException: Table/View 'UPDATECLOB' already exists in Schema 'APP'.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45)
          at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:201)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:391)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346)
          at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2201)
          at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:81)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1323)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest.setUp(ClobUpdatableReaderTest.java:284)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          Caused by: ERROR X0Y32: Table/View 'UPDATECLOB' already exists in Schema 'APP'.
          at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:373)
          at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.duplicateDescriptorException(DataDictionaryImpl.java:1817)
          at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.addDescriptor(DataDictionaryImpl.java:1808)
          at org.apache.derby.impl.sql.execute.CreateTableConstantAction.executeConstantAction(CreateTableConstantAction.java:238)
          at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:416)
          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:297)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1235)
          ... 29 more
          4) testAttributeAccumulatedConnectionCount(org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest)java.security.PrivilegedActionException: javax.management.InstanceNotFoundException: org.apache.derby:type=NetworkServer,system=c013800d-011d-78ed-5899-ffffe1d7aa3e
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.getAttribute(MBeanTest.java:379)
          at org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest.testAttributeAccumulatedConnectionCount(NetworkServerMBeanTest.java:93)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          Caused by: javax.management.InstanceNotFoundException: org.apache.derby:type=NetworkServer,system=c013800d-011d-78ed-5899-ffffe1d7aa3e
          at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getMBean(DefaultMBeanServerInterceptor.java:1010)
          at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getAttribute(DefaultMBeanServerInterceptor.java:627)
          at com.sun.jmx.mbeanserver.JmxMBeanServer.getAttribute(JmxMBeanServer.java:659)
          at org.apache.derbyTesting.functionTests.tests.management.MBeanTest$4.run(MBeanTest.java:382)
          ... 41 more
          There was 1 failure:
          1) testStressMulti(org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest)junit.framework.AssertionFailedError: Caused by:
          java.sql.SQLException: Java exception: 'ASSERT FAILED transaction table has null entry: org.apache.derby.shared.common.sanity.AssertFailure'.
          at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45)
          at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:87)
          at org.apache.derby.impl.jdbc.Util.javaException(Util.java:244)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:403)
          at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346)
          at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2201)
          at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:81)
          at org.apache.derby.impl.jdbc.EmbedResultSet.closeOnTransactionError(EmbedResultSet.java:4321)
          at org.apache.derby.impl.jdbc.EmbedResultSet.movePosition(EmbedResultSet.java:463)
          at org.apache.derby.impl.jdbc.EmbedResultSet.next(EmbedResultSet.java:367)
          at org.apache.derbyTesting.junit.JDBC.assertDrainResults(JDBC.java:627)
          at org.apache.derbyTesting.junit.JDBC.assertDrainResults(JDBC.java:604)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.select(StressMultiTest.java:536)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:409)
          at java.lang.Thread.run(Thread.java:613)
          Caused by: org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED transaction table has null entry
          at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityManager.java:120)
          at org.apache.derby.impl.store.raw.xact.TransactionTable.getTransactionInfo(TransactionTable.java:968)
          at org.apache.derby.impl.store.raw.xact.XactFactory.getTransactionInfo(XactFactory.java:991)
          at org.apache.derby.impl.store.raw.RawStore.getTransactionInfo(RawStore.java:1158)
          at org.apache.derby.impl.store.access.RAMAccessManager.getTransactionInfo(RAMAccessManager.java:912)
          at org.apache.derby.impl.services.locks.Deadlock.buildException(Deadlock.java:266)
          at org.apache.derby.impl.services.locks.ConcurrentLockSet.lockObject(ConcurrentLockSet.java:613)
          at org.apache.derby.impl.services.locks.ConcurrentLockSet.zeroDurationLockObject(ConcurrentLockSet.java:855)
          at org.apache.derby.impl.services.locks.AbstractPool.zeroDurationlockObject(AbstractPool.java:297)
          at org.apache.derby.impl.store.raw.xact.RowLocking2nohold.lockRecordForRead(RowLocking2nohold.java:89)
          at org.apache.derby.impl.store.access.conglomerate.OpenConglomerate.lockPositionForRead(OpenConglomerate.java:436)
          at org.apache.derby.impl.store.access.conglomerate.GenericScanController.fetchRows(GenericScanController.java:683)
          at org.apache.derby.impl.store.access.heap.HeapScan.fetchNextGroup(HeapScan.java:324)
          at org.apache.derby.impl.sql.execute.BulkTableScanResultSet.reloadArray(BulkTableScanResultSet.java:327)
          at org.apache.derby.impl.sql.execute.BulkTableScanResultSet.getNextRowCore(BulkTableScanResultSet.java:282)
          at org.apache.derby.impl.sql.execute.BasicNoPutResultSetImpl.getNextRow(BasicNoPutResultSetImpl.java:460)
          at org.apache.derby.impl.jdbc.EmbedResultSet.movePosition(EmbedResultSet.java:423)
          ... 6 more

          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.handleException(StressMultiTest.java:331)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.access$200(StressMultiTest.java:70)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:425)
          at java.lang.Thread.run(Thread.java:613)

          FAILURES!!!
          Tests run: 8888, Failures: 1, Errors: 4

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me on derby-481-14-ab-dropColumn.diff except for 4 errors. These errors were in ClobUpdatableReaderTest, NetworkServerMBeanTest, and StressMultiTest. I think these are either errors introduced by recent submissions or outstanding heisenbugs: There were 4 errors: 1) testUpdateableStoreReader(org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest)java.sql.SQLException: Java exception: 'Bad file descriptor: java.io.IOException'. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45) at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:87) at org.apache.derby.impl.jdbc.Util.javaException(Util.java:244) at org.apache.derby.impl.jdbc.EmbedConnection.clearLOBMapping(EmbedConnection.java:3097) at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1765) at org.apache.derbyTesting.junit.BaseJDBCTestCase.rollback(BaseJDBCTestCase.java:378) at org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest.tearDown(ClobUpdatableReaderTest.java:304) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) Caused by: java.io.IOException: Bad file descriptor at java.io.RandomAccessFile.close0(Native Method) at java.io.RandomAccessFile.close(RandomAccessFile.java:532) at org.apache.derby.impl.jdbc.LOBFile.close(LOBFile.java:148) at org.apache.derby.impl.jdbc.EmbedConnection.clearLOBMapping(EmbedConnection.java:3095) ... 29 more 2) testUpdateableReader(org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest)java.sql.SQLException: Table/View 'UPDATECLOB' already exists in Schema 'APP'. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45) at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:201) at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:391) at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346) at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2201) at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:81) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1323) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555) at org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest.setUp(ClobUpdatableReaderTest.java:284) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) Caused by: ERROR X0Y32: Table/View 'UPDATECLOB' already exists in Schema 'APP'. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:373) at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.duplicateDescriptorException(DataDictionaryImpl.java:1817) at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.addDescriptor(DataDictionaryImpl.java:1808) at org.apache.derby.impl.sql.execute.CreateTableConstantAction.executeConstantAction(CreateTableConstantAction.java:238) at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:416) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:297) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1235) ... 29 more 3) testMultiplexedOperationProblem(org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest)java.sql.SQLException: Table/View 'UPDATECLOB' already exists in Schema 'APP'. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45) at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Util.java:201) at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:391) at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346) at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2201) at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:81) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1323) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555) at org.apache.derbyTesting.functionTests.tests.jdbcapi.ClobUpdatableReaderTest.setUp(ClobUpdatableReaderTest.java:284) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) Caused by: ERROR X0Y32: Table/View 'UPDATECLOB' already exists in Schema 'APP'. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:373) at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.duplicateDescriptorException(DataDictionaryImpl.java:1817) at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.addDescriptor(DataDictionaryImpl.java:1808) at org.apache.derby.impl.sql.execute.CreateTableConstantAction.executeConstantAction(CreateTableConstantAction.java:238) at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:416) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:297) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1235) ... 29 more 4) testAttributeAccumulatedConnectionCount(org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest)java.security.PrivilegedActionException: javax.management.InstanceNotFoundException: org.apache.derby:type=NetworkServer,system=c013800d-011d-78ed-5899-ffffe1d7aa3e at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.getAttribute(MBeanTest.java:379) at org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest.testAttributeAccumulatedConnectionCount(NetworkServerMBeanTest.java:93) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:105) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) Caused by: javax.management.InstanceNotFoundException: org.apache.derby:type=NetworkServer,system=c013800d-011d-78ed-5899-ffffe1d7aa3e at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getMBean(DefaultMBeanServerInterceptor.java:1010) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getAttribute(DefaultMBeanServerInterceptor.java:627) at com.sun.jmx.mbeanserver.JmxMBeanServer.getAttribute(JmxMBeanServer.java:659) at org.apache.derbyTesting.functionTests.tests.management.MBeanTest$4.run(MBeanTest.java:382) ... 41 more There was 1 failure: 1) testStressMulti(org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest)junit.framework.AssertionFailedError: Caused by: java.sql.SQLException: Java exception: 'ASSERT FAILED transaction table has null entry: org.apache.derby.shared.common.sanity.AssertFailure'. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45) at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:87) at org.apache.derby.impl.jdbc.Util.javaException(Util.java:244) at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:403) at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346) at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:2201) at org.apache.derby.impl.jdbc.ConnectionChild.handleException(ConnectionChild.java:81) at org.apache.derby.impl.jdbc.EmbedResultSet.closeOnTransactionError(EmbedResultSet.java:4321) at org.apache.derby.impl.jdbc.EmbedResultSet.movePosition(EmbedResultSet.java:463) at org.apache.derby.impl.jdbc.EmbedResultSet.next(EmbedResultSet.java:367) at org.apache.derbyTesting.junit.JDBC.assertDrainResults(JDBC.java:627) at org.apache.derbyTesting.junit.JDBC.assertDrainResults(JDBC.java:604) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.select(StressMultiTest.java:536) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:409) at java.lang.Thread.run(Thread.java:613) Caused by: org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED transaction table has null entry at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityManager.java:120) at org.apache.derby.impl.store.raw.xact.TransactionTable.getTransactionInfo(TransactionTable.java:968) at org.apache.derby.impl.store.raw.xact.XactFactory.getTransactionInfo(XactFactory.java:991) at org.apache.derby.impl.store.raw.RawStore.getTransactionInfo(RawStore.java:1158) at org.apache.derby.impl.store.access.RAMAccessManager.getTransactionInfo(RAMAccessManager.java:912) at org.apache.derby.impl.services.locks.Deadlock.buildException(Deadlock.java:266) at org.apache.derby.impl.services.locks.ConcurrentLockSet.lockObject(ConcurrentLockSet.java:613) at org.apache.derby.impl.services.locks.ConcurrentLockSet.zeroDurationLockObject(ConcurrentLockSet.java:855) at org.apache.derby.impl.services.locks.AbstractPool.zeroDurationlockObject(AbstractPool.java:297) at org.apache.derby.impl.store.raw.xact.RowLocking2nohold.lockRecordForRead(RowLocking2nohold.java:89) at org.apache.derby.impl.store.access.conglomerate.OpenConglomerate.lockPositionForRead(OpenConglomerate.java:436) at org.apache.derby.impl.store.access.conglomerate.GenericScanController.fetchRows(GenericScanController.java:683) at org.apache.derby.impl.store.access.heap.HeapScan.fetchNextGroup(HeapScan.java:324) at org.apache.derby.impl.sql.execute.BulkTableScanResultSet.reloadArray(BulkTableScanResultSet.java:327) at org.apache.derby.impl.sql.execute.BulkTableScanResultSet.getNextRowCore(BulkTableScanResultSet.java:282) at org.apache.derby.impl.sql.execute.BasicNoPutResultSetImpl.getNextRow(BasicNoPutResultSetImpl.java:460) at org.apache.derby.impl.jdbc.EmbedResultSet.movePosition(EmbedResultSet.java:423) ... 6 more at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.handleException(StressMultiTest.java:331) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.access$200(StressMultiTest.java:70) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:425) at java.lang.Thread.run(Thread.java:613) FAILURES!!! Tests run: 8888, Failures: 1, Errors: 4
          Rick Hillegas made changes -
          Attachment derby-481-14-ab-dropColumn.diff [ 12393523 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-14-ab-dropColumn.diff. Running tests now. This patch implements CASCADEd and RESTRICTed drops of columns which are referenced by generated columns, per the spec. In short, if a generated column depends on the column being dropped, then RESTRICT aborts the drop and CASCADE causes the generated column to be dropped along with the original column. Makes the following changes:

          A) Reworks the implementation of the default descriptor so that it contains an array of referenced column names rather than column positions. This eliminates the need to remap those column positions when other columns are dropped.

          B) Makes the drop column logic recursively call itself for generated columns which depend on the original column being dropped.

          Touches the following files:

          M java/engine/org/apache/derby/catalog/DefaultInfo.java
          M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java

          Changes to the default descriptor needed for (A).

          M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java
          M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java

          Accomodations for (A) in code which uses default descriptors.

          M java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java

          Reworks dropColumnFromTable() so that it calls itself on affected generated columns.

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

          New error message and warning.

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

          New tests for ALTER TABLE DROP COLUMN scenarios involving generated columns.

          Show
          Rick Hillegas added a comment - Attaching derby-481-14-ab-dropColumn.diff. Running tests now. This patch implements CASCADEd and RESTRICTed drops of columns which are referenced by generated columns, per the spec. In short, if a generated column depends on the column being dropped, then RESTRICT aborts the drop and CASCADE causes the generated column to be dropped along with the original column. Makes the following changes: A) Reworks the implementation of the default descriptor so that it contains an array of referenced column names rather than column positions. This eliminates the need to remap those column positions when other columns are dropped. B) Makes the drop column logic recursively call itself for generated columns which depend on the original column being dropped. Touches the following files: M java/engine/org/apache/derby/catalog/DefaultInfo.java M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java Changes to the default descriptor needed for (A). M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java Accomodations for (A) in code which uses default descriptors. M java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java Reworks dropColumnFromTable() so that it calls itself on affected generated columns. M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java New error message and warning. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java New tests for ALTER TABLE DROP COLUMN scenarios involving generated columns.
          Rick Hillegas made changes -
          Link This issue is related to DERBY-3940 [ DERBY-3940 ]
          Rick Hillegas made changes -
          Attachment derby-481-13-aa-alterDatatype.diff [ 12393390 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-13-aa-alterDatatype.diff. This patch adds tests for changing the datatype of generated columns and of the columns they reference. Committed at subversion revision 711663.

          Show
          Rick Hillegas added a comment - Attaching derby-481-13-aa-alterDatatype.diff. This patch adds tests for changing the datatype of generated columns and of the columns they reference. Committed at subversion revision 711663.
          Rick Hillegas made changes -
          Attachment derby-481-12-aa-padding.diff [ 12393376 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-12-aa-padding.diff. This adds some more tests of generated columns involving different length character and numeric columns. Committed at subversion revision 711579. Touches the following file:

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

          Show
          Rick Hillegas added a comment - Attaching derby-481-12-aa-padding.diff. This adds some more tests of generated columns involving different length character and numeric columns. Committed at subversion revision 711579. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java
          Hide
          Rick Hillegas added a comment -

          Tests against derby-481-11-aa-notNull.diff ran cleanly for me except for the heisenbugs in StressMultiTest. Committed at subversion revision 711571.

          Show
          Rick Hillegas added a comment - Tests against derby-481-11-aa-notNull.diff ran cleanly for me except for the heisenbugs in StressMultiTest. Committed at subversion revision 711571.
          Rick Hillegas made changes -
          Attachment derby-481-11-aa-notNull.diff [ 12393330 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-11-aa-notNull.diff. Running tests now. This patch adds support for NOT NULL constraints on generated columns. Makes the following changes:

          A) Exposes the "normalization" logic in NormalizeResultSet and applies this logic to generated columns. This logic is what enforces NOT NULL constraints. This logic was already being called by the INSERT and UPDATE execution-time logic after populating the target row. Using the terms introduced above with the summary of patch derby-481-04-aa-insert.diff: the normalization logic was being called immediately after (1') executed but it wasn't being called after (2') was executed. Now this logic is also called on generated columns after they are populated--that is, after (2') runs.

          B) Loosens a condition which prevents ALTER TABLE from adding new columns which don't have DEFAULT clauses. Now it's ok to add these clauses with NOT NULL if they are generated columns.

          C) Adds some tests to verify (A) and (B).

          Touches the following files:

          M java/engine/org/apache/derby/impl/sql/execute/NormalizeResultSet.java

          Exposes the "normalization" logic which is called after (1') runs.

          M java/engine/org/apache/derby/impl/sql/GenericColumnDescriptor.java
          M java/engine/org/apache/derby/iapi/sql/ResultColumnDescriptor.java

          Adds a new method to ResultColumnDescriptors so that they can be asked whether they represent generated columns. This is useful so that we only call the normalization logic on generated columns rather than all columns in the target row.

          M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java

          Calls the normalization logic on generated columns just after (2') runs. Slightly changes the signature of evaluateGenerationClauses().

          M java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java
          M java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java

          Reworks calls to evaluateGenerationClauses().

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

          Changes the ALTER TABLE logic for (B).

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

          Adds new tests.

          Show
          Rick Hillegas added a comment - Attaching derby-481-11-aa-notNull.diff. Running tests now. This patch adds support for NOT NULL constraints on generated columns. Makes the following changes: A) Exposes the "normalization" logic in NormalizeResultSet and applies this logic to generated columns. This logic is what enforces NOT NULL constraints. This logic was already being called by the INSERT and UPDATE execution-time logic after populating the target row. Using the terms introduced above with the summary of patch derby-481-04-aa-insert.diff: the normalization logic was being called immediately after (1') executed but it wasn't being called after (2') was executed. Now this logic is also called on generated columns after they are populated--that is, after (2') runs. B) Loosens a condition which prevents ALTER TABLE from adding new columns which don't have DEFAULT clauses. Now it's ok to add these clauses with NOT NULL if they are generated columns. C) Adds some tests to verify (A) and (B). Touches the following files: M java/engine/org/apache/derby/impl/sql/execute/NormalizeResultSet.java Exposes the "normalization" logic which is called after (1') runs. M java/engine/org/apache/derby/impl/sql/GenericColumnDescriptor.java M java/engine/org/apache/derby/iapi/sql/ResultColumnDescriptor.java Adds a new method to ResultColumnDescriptors so that they can be asked whether they represent generated columns. This is useful so that we only call the normalization logic on generated columns rather than all columns in the target row. M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java Calls the normalization logic on generated columns just after (2') runs. Slightly changes the signature of evaluateGenerationClauses(). M java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java M java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java Reworks calls to evaluateGenerationClauses(). M java/engine/org/apache/derby/impl/sql/compile/ColumnDefinitionNode.java Changes the ALTER TABLE logic for (B). M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Adds new tests.
          Hide
          Rick Hillegas added a comment -

          Committed derby-481-10-aa-foreignKeyActions.diff at subversion revision 711135.

          Show
          Rick Hillegas added a comment - Committed derby-481-10-aa-foreignKeyActions.diff at subversion revision 711135.
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me on deby-481-10-aa-foreignKeyActions.diff.

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me on deby-481-10-aa-foreignKeyActions.diff.
          Rick Hillegas made changes -
          Attachment derby-481-10-aa-foreignKeyActions.diff [ 12393260 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-10-aa-foreignKeyActions.diff. Running tests now. This patch addresses the second issue which Dag raised, preventing users from adding foreign keys on generated columns when the foreign keys have SET NULL or SET DEFAULT delete actions.

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

          Adds a method to identify and raise exceptions on these forbidden foreign keys.

          M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java
          M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java

          Wires the new method into the bind-time logic for CREATE/ALTER TABLE.

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

          Adds a new error message for the illegal foreign keys.

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

          Adds tests for the illegal foreign keys.

          Show
          Rick Hillegas added a comment - Attaching derby-481-10-aa-foreignKeyActions.diff. Running tests now. This patch addresses the second issue which Dag raised, preventing users from adding foreign keys on generated columns when the foreign keys have SET NULL or SET DEFAULT delete actions. M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java Adds a method to identify and raise exceptions on these forbidden foreign keys. M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java Wires the new method into the bind-time logic for CREATE/ALTER TABLE. M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java Adds a new error message for the illegal foreign keys. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Adds tests for the illegal foreign keys.
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me on derby-481-09-aa-dummyDefaults.diff. Committed at subversion revision 710071.

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me on derby-481-09-aa-dummyDefaults.diff. Committed at subversion revision 710071.
          Rick Hillegas made changes -
          Attachment derby-481-09-aa-dummyDefaults.diff [ 12393178 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-09-aa-dummyDefaults.diff. This patch addresses a bug found by Dag: If you tried to poke the literal DEFAULT into a generated column using an INSERT without a target list (meaning all values in the row had to be specified), then we bombed trying to bind the generation clause prematurely.

          Touches the following files:

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

          At the point where we substitute default values for DEFAULT literals, we just substitute an untyped NULL for generation clauses. Later on, when we have enough context to bind the generation clauses, we replace these NULLs with the actual generation clauses.

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

          Tests for the problem case which Dag found.

          Show
          Rick Hillegas added a comment - Attaching derby-481-09-aa-dummyDefaults.diff. This patch addresses a bug found by Dag: If you tried to poke the literal DEFAULT into a generated column using an INSERT without a target list (meaning all values in the row had to be specified), then we bombed trying to bind the generation clause prematurely. Touches the following files: M java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java At the point where we substitute default values for DEFAULT literals, we just substitute an untyped NULL for generation clauses. Later on, when we have enough context to bind the generation clauses, we replace these NULLs with the actual generation clauses. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Tests for the problem case which Dag found.
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me on derby-481-aa-castToDeclaredType.diff. Committed at subversion revision 709577.

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me on derby-481-aa-castToDeclaredType.diff. Committed at subversion revision 709577.
          Rick Hillegas made changes -
          Attachment derby-481-08-aa-castToDeclaredType.diff [ 12393173 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-08-aa-castToDeclaredType.diff. Running regression tests now.

          This patch adds a run-time cast on top of the generation clause in order to guarantee that generation clauses are correctly converted to the declared type of their columns per the functional spec. Touches the following files:

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

          Adds the cast.

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

          Adds tests verifying that generation clauses are correctly assigned to declared types for all of the assignments documented in the Reference Guide section titled "Data type assignments and comparison, sorting, and ordering".

          Show
          Rick Hillegas added a comment - Attaching derby-481-08-aa-castToDeclaredType.diff. Running regression tests now. This patch adds a run-time cast on top of the generation clause in order to guarantee that generation clauses are correctly converted to the declared type of their columns per the functional spec. Touches the following files: M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java Adds the cast. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Adds tests verifying that generation clauses are correctly assigned to declared types for all of the assignments documented in the Reference Guide section titled "Data type assignments and comparison, sorting, and ordering".
          Hide
          Rick Hillegas added a comment -

          Hi Dag,

          Thanks for digging into this!

          1) "insert into bar values (2, default); – fails trying to bind "i" " - This looks like a bug to me.

          2) I didn't notice section 11.8, SR 12a) when I wrote the functional spec. I should add that to the functional spec and make sure that those cases are forbidden.

          Thanks!

          Show
          Rick Hillegas added a comment - Hi Dag, Thanks for digging into this! 1) "insert into bar values (2, default); – fails trying to bind "i" " - This looks like a bug to me. 2) I didn't notice section 11.8, SR 12a) when I wrote the functional spec. I should add that to the functional spec and make sure that those cases are forbidden. Thanks!
          Hide
          Dag H. Wanvik added a comment -

          I tried this fragment which seems to be forbidden by section 11.8, SR 12a), p.549:

          create table bar2 (i int, j int generated always as (i*2) references foo on delete set null);

          The "set null" part here is not flagged, but probably should be (conflict with generated clause)?
          Sorry if this is pending work, I am just working my way through the standard

          Show
          Dag H. Wanvik added a comment - I tried this fragment which seems to be forbidden by section 11.8, SR 12a), p.549: create table bar2 (i int, j int generated always as (i*2) references foo on delete set null); The "set null" part here is not flagged, but probably should be (conflict with generated clause)? Sorry if this is pending work, I am just working my way through the standard
          Hide
          Dag H. Wanvik added a comment -

          Started playing a bit with generated columns, this behavior seems wrong?

          create table bar (i int, j int generated always as (-i));
          insert into bar(i,j) values (1, default); – works
          insert into bar values (2, default); – fails trying to bind "i"

          ERROR 42X04: Column 'I' is either not in any table in the FROM list or appears within a join specification and is outside the scope of the join specification or appears in a HAVING clause and is not in the GROUP BY list. If this is a CREATE or ALTER TABLE statement then 'I' is not a column in the target table.

          Show
          Dag H. Wanvik added a comment - Started playing a bit with generated columns, this behavior seems wrong? create table bar (i int, j int generated always as (-i)); insert into bar(i,j) values (1, default); – works insert into bar values (2, default); – fails trying to bind "i" ERROR 42X04: Column 'I' is either not in any table in the FROM list or appears within a join specification and is outside the scope of the join specification or appears in a HAVING clause and is not in the GROUP BY list. If this is a CREATE or ALTER TABLE statement then 'I' is not a column in the target table.
          Rick Hillegas made changes -
          Attachment derby-481-07-ab-noSQLinRoutines.diff [ 12393154 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-07-ab-noSQLinRoutines.diff. This is identical to the previous patch except that it adds negative tests for MAX, MIN, and COUNT aggregates in generation clauses. Tests run cleanly for me. Committed at subversion revision 709415.

          Show
          Rick Hillegas added a comment - Attaching derby-481-07-ab-noSQLinRoutines.diff. This is identical to the previous patch except that it adds negative tests for MAX, MIN, and COUNT aggregates in generation clauses. Tests run cleanly for me. Committed at subversion revision 709415.
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3932 [ DERBY-3932 ]
          Rick Hillegas made changes -
          Attachment derby-481-07-aa-noSQLinRoutines.diff [ 12393106 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-07-aa-noSQLinRoutines.diff. Tests are running now. This patch makes the following changes:

          1) Rejects generation clauses which contain user functions which may contain SQL, per the functional spec.

          2) Adds tests for the above change.

          3) Adds tests to verify that aggregates are not allowed in generation clauses.

          4) Adds tests to verify that generation clauses may not contain unstable system functions like current_user (the functions listed in Appendix A of the functional spec). Tests don't yet verify that you can't include current_role--to test that we need another test which runs with authorization turned on.

          Touches the following files:

          M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java
          M java/engine/org/apache/derby/loc/messages.xml
          M java/shared/org/apache/derby/shared/common/reference/SQLState.java

          Adds new error message which objects that generation clauses may not contain sql-issuing functions.

          M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java
          M java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java

          Wires in the prohibition in (1).

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

          Adds new tests.

          Show
          Rick Hillegas added a comment - Attaching derby-481-07-aa-noSQLinRoutines.diff. Tests are running now. This patch makes the following changes: 1) Rejects generation clauses which contain user functions which may contain SQL, per the functional spec. 2) Adds tests for the above change. 3) Adds tests to verify that aggregates are not allowed in generation clauses. 4) Adds tests to verify that generation clauses may not contain unstable system functions like current_user (the functions listed in Appendix A of the functional spec). Tests don't yet verify that you can't include current_role--to test that we need another test which runs with authorization turned on. Touches the following files: M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java Adds new error message which objects that generation clauses may not contain sql-issuing functions. M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java M java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java Wires in the prohibition in (1). M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Adds new tests.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-481-06-aa-genReferences.diff. Committed at subversion revision 709219.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-481-06-aa-genReferences.diff. Committed at subversion revision 709219.
          Rick Hillegas made changes -
          Attachment derby-481-06-aa-genreferences.diff [ 12393065 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-06-aa-genReferences.diff. This patch makes it illegal for generated columns to reference other generated columns, per the functional spec. Running regression tests now.

          Touches the following files:

          M java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java

          Adds some small helper methods to assist the machinery in TableElementList.bindAndValidateGenerationClauses.

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

          Adds logic to raise an exception if a generation clause mentions generated columns.

          M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java
          M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java

          Adds new argument to calls to TableElementList.bindAndValidateGenerationClauses.

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

          Adds new error message raised when generation clauses reference generated columns.

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

          Adds tests for this behavior. While I was in there, I added tests to verify some other functionality: generation clauses can't reference other tables or run subqueries.

          Show
          Rick Hillegas added a comment - Attaching derby-481-06-aa-genReferences.diff. This patch makes it illegal for generated columns to reference other generated columns, per the functional spec. Running regression tests now. Touches the following files: M java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java Adds some small helper methods to assist the machinery in TableElementList.bindAndValidateGenerationClauses. M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java Adds logic to raise an exception if a generation clause mentions generated columns. M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java Adds new argument to calls to TableElementList.bindAndValidateGenerationClauses. M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java Adds new error message raised when generation clauses reference generated columns. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Adds tests for this behavior. While I was in there, I added tests to verify some other functionality: generation clauses can't reference other tables or run subqueries.
          Hide
          Rick Hillegas added a comment -

          Committed derby-481-05-aa-update.diff at subversion revision 708900.

          Show
          Rick Hillegas added a comment - Committed derby-481-05-aa-update.diff at subversion revision 708900.
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me on the derby-481-05-aa-update.diff patch.

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me on the derby-481-05-aa-update.diff patch.
          Rick Hillegas made changes -
          Attachment derby-481-05-aa-update.diff [ 12392931 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-05-aa-update.diff, the last chunk from the prototype patch. I am running regression tests now.

          This patch wires in UPDATE support for generated columns. As with INSERT, I threaded my way through the UPDATE machinery largely by following the way that CHECK constraints are handled.

          UPDATE presents a couple extra issues in addition to the issues raised by INSERT:

          1) We don't have to enhance the target row with defaulted columns. Instead, we have to enhance the update list with all generated columns whose generation clauses reference columns in the update list.

          2) If a generated column is added to the update list, then the columns it references must be added to the list of columns which are read from the base row.

          3) The target row for UPDATE holds two copies of each column that either has to be written or read: the before value of the column and the after value. This gives rise to some trickiness.

          Touches the following files:

          ------------------------------------------------------------------
          – BINDING AND CODE GENERATION
          ------------------------------------------------------------------

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

          Adds generated columns to the update list if they are affected by the update. For all generated columns in the update list, adds the columns they reference to the list of columns to be read from disk. Forbids the setting of a generated column to anything except the DEFAULT literal. Generates a method to fill in computed columns at execution time--this is the (2') method described above in the summary of patch derby-04-aa-insert.diff.

          ------------------------------------------------------------------
          – EXECUTION
          ------------------------------------------------------------------

          M java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java
          M java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java

          Adds the (2') method as an argument to the factory methods that create UpdateResultSets.

          M java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java

          Invokes (2') close to where we invoke (3') already.

          ------------------------------------------------------------------
          – TESTS
          ------------------------------------------------------------------

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

          Uncommented basic tests for updates. Uncommented basic tests for the interaction of generated columns with triggers and foreign keys.

          Show
          Rick Hillegas added a comment - Attaching derby-481-05-aa-update.diff, the last chunk from the prototype patch. I am running regression tests now. This patch wires in UPDATE support for generated columns. As with INSERT, I threaded my way through the UPDATE machinery largely by following the way that CHECK constraints are handled. UPDATE presents a couple extra issues in addition to the issues raised by INSERT: 1) We don't have to enhance the target row with defaulted columns. Instead, we have to enhance the update list with all generated columns whose generation clauses reference columns in the update list. 2) If a generated column is added to the update list, then the columns it references must be added to the list of columns which are read from the base row. 3) The target row for UPDATE holds two copies of each column that either has to be written or read: the before value of the column and the after value. This gives rise to some trickiness. Touches the following files: ------------------------------------------------------------------ – BINDING AND CODE GENERATION ------------------------------------------------------------------ M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java Adds generated columns to the update list if they are affected by the update. For all generated columns in the update list, adds the columns they reference to the list of columns to be read from disk. Forbids the setting of a generated column to anything except the DEFAULT literal. Generates a method to fill in computed columns at execution time--this is the (2') method described above in the summary of patch derby-04-aa-insert.diff. ------------------------------------------------------------------ – EXECUTION ------------------------------------------------------------------ M java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java M java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java Adds the (2') method as an argument to the factory methods that create UpdateResultSets. M java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java Invokes (2') close to where we invoke (3') already. ------------------------------------------------------------------ – TESTS ------------------------------------------------------------------ M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Uncommented basic tests for updates. Uncommented basic tests for the interaction of generated columns with triggers and foreign keys.
          Hide
          Rick Hillegas added a comment -

          Thanks for puzzling through the patch summary, Bryan. I appreciate the sanity-check. I committed derby-481-04-aa-insert.diff at subversion revision 708561.

          Show
          Rick Hillegas added a comment - Thanks for puzzling through the patch summary, Bryan. I appreciate the sanity-check. I committed derby-481-04-aa-insert.diff at subversion revision 708561.
          Hide
          Bryan Pendleton added a comment -

          Modeling the overall execution behaviors on CHECK constraints seems like
          a reasonable approach to me. Thanks for taking the time to explain your
          thinking. It seems like you've found a couple of thorny implementation details,
          your strategy of addressing them seems fine to me.

          Show
          Bryan Pendleton added a comment - Modeling the overall execution behaviors on CHECK constraints seems like a reasonable approach to me. Thanks for taking the time to explain your thinking. It seems like you've found a couple of thorny implementation details, your strategy of addressing them seems fine to me.
          Hide
          Rick Hillegas added a comment -

          Tests pass cleanly for me on derby-481-04-aa-insert.diff.

          Show
          Rick Hillegas added a comment - Tests pass cleanly for me on derby-481-04-aa-insert.diff.
          Rick Hillegas made changes -
          Attachment derby-481-04-aa-insert.diff [ 12392871 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-04-aa-insert.diff. I am running regression tests now.

          This patch wires in INSERT support for generated columns. I threaded my way through the INSERT machinery largely by following the way that CHECK constraints are handled.

          Before this patch, the compiler built 2 significant methods for evaluating expressions:

          1) A method which populates the base row from whatever data source is driving the INSERT. That data source could be, for instance, a list of literal values or a SELECT statement.

          2) A method which runs the CHECK constraints.

          My first attempt to support INSERT involved building the generation clauses into method (1). Unfortunately, that method is generated by the data sources, not by the driving INSERT node. I got this approach to work for the degenerate case of inserting a single literal value. But this approach failed when I tried to insert multiple literal values (where the data source is a UNION) and it failed when the data source was a SELECT. It became apparent that this approach would involve wiring code-generation logic into all implementations of ResultSet--there are quite a few. This began to look too complicated so I abandoned this approach.

          The current patch represents a second attempt. Here the approach is to give the generation clauses their own method. Now the compiler builds 3 significant methods for evaluating expressions:

          1') The original method which populates the base row from a data source (see above).

          2') A new method which runs the generation clauses, looking for referenced columns in the row built by (1') and poking the generated values into that row.

          3') The original method which runs the CHECK constraints (see above).

          That was the tricky bit for compilation.

          The tricky bit for execution was this: the base row has to be poked into the Activation so that it is visible to the generation clauses when (2') runs. A similar poking is done for CHECK constraints. If you examine this poking for CHECK constraints, you will notice that sometimes the poking is undone after the constraints run and sometimes we don't bother to undo the poking. I don't understand the difference between these code paths. As a result, I have defensively coded the new poking which we need for generated columns. I poke the base row into the Activation just before the generation clauses run. After the generation clauses run, I return the Activation to its previous state.

          Here is a little more detail on the implementation:

          A) At bind() time we do the following:

          i) Prune out explicit mentions of generated columns. These can arise if the user sets a generated column to the literal DEFAULT--as allowed by the ANSI/ISO syntax. So for instance, the following is legal:

          insert into T( refCol, generatedCol ) values ( 1, default )

          We prune out the explicitly added generated columns because, later on in the bind() phase, the insert list is expanded to include all columns with defaults (not just generated columns).

          ii) When the insert list is expanded to include all defaulted columns, we add in the generated columns but we don't bind their expressions. This is because the generation clause may refer to other columns in the base row. This, in turn, creates an ordering problem. In addition we we don't yet have a result set number for the base row--we need that number in order to bind references to other columns which may appear in the generation clauses.

          iii) Later on, just before we parse and bind the CHECK constraints, we parse and bind the generation clauses. At this point, we have enough context to bind the referenced columns.

          B) At generate() time, we generate method (2') in between generating (1') and (3'). The generated (2') method is now one of the arguments to the factory method which creates the execution-driver, the InsertResultSet. This is just like what we do for CHECK constraints: the generated (3') method is also an argument to the instantiation of the InsertResultSet.

          C) At execution time, we evaluate (2') just before we evaluate (3').

          Touches the following files:

          --------------------
          – BINDING
          --------------------

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

          Adds a method so that a ResultColumn can report whether it represents a generated column. I also forced all overrides of the expression field to go through the setExpression() method. This, technically speaking, is not necessary--but it made debugging easier for me and I think it will be useful for other developers who need to debug this node.

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

          Changes are made to support both binding and code-generation. These are the bind() changes:

          i) Adds a method to object if the user tries to override the value in a generated column with any value other than the DEFAULT literal. For instance, the following is illegal:

          insert into T( refCol, generatedCol ) values ( 1, 70 )

          In addition, we remove explicit mentions of generated columns because we will add them back when we enhance the INSERT statement with defaulted columns.

          ii) Adds logic to parse and bind generated columns. This is modelled on the logic which parses and binds CHECK constraints.

          iii) Renames bindCheckConstraint() to bindRowScopedExpression() because this method is now shared by the logic which binds CHECK constraints and the logic which binds generation clauses.

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

          Short-circuits the logic which enhances the base row with defaulted columns. Adds in the generated columns but does not add their generation clauses. This is because the clauses cannot be bound at the same time as the rest of the columns in the base row. We wait to bind them until the time that we bind CHECK constraints.

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

          Wires binding and code-generation calls into bindStatement() and generate().

          --------------------
          – CODE GENERATION
          --------------------

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

          Skips code-generation for generated columns when walking the base row. The generateCore() method generates (1'). We need to build the generation clauses into (2') instead and this is done later on.

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

          In addition to the bind() changes described above, adds logic to generate the (2') method.

          --------------------
          – EXECUTION
          --------------------

          M java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java
          M java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java

          Adds (2') as an argument to the factory method which instantiates InsertResultSets.

          M java/engine/org/apache/derby/iapi/sql/Activation.java
          M java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java
          M java/engine/org/apache/derby/impl/sql/GenericActivationHolder.java

          Adds a method for retrieving the current row from the Activation. This allows us to return the Activation to its original state after we have run (2').

          M java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java
          M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java

          Evaluates generation clauses close to where CHECK constraints are evaluated.

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

          Uncomments basic INSERT tests.

          Show
          Rick Hillegas added a comment - Attaching derby-481-04-aa-insert.diff. I am running regression tests now. This patch wires in INSERT support for generated columns. I threaded my way through the INSERT machinery largely by following the way that CHECK constraints are handled. Before this patch, the compiler built 2 significant methods for evaluating expressions: 1) A method which populates the base row from whatever data source is driving the INSERT. That data source could be, for instance, a list of literal values or a SELECT statement. 2) A method which runs the CHECK constraints. My first attempt to support INSERT involved building the generation clauses into method (1). Unfortunately, that method is generated by the data sources, not by the driving INSERT node. I got this approach to work for the degenerate case of inserting a single literal value. But this approach failed when I tried to insert multiple literal values (where the data source is a UNION) and it failed when the data source was a SELECT. It became apparent that this approach would involve wiring code-generation logic into all implementations of ResultSet--there are quite a few. This began to look too complicated so I abandoned this approach. The current patch represents a second attempt. Here the approach is to give the generation clauses their own method. Now the compiler builds 3 significant methods for evaluating expressions: 1') The original method which populates the base row from a data source (see above). 2') A new method which runs the generation clauses, looking for referenced columns in the row built by (1') and poking the generated values into that row. 3') The original method which runs the CHECK constraints (see above). That was the tricky bit for compilation. The tricky bit for execution was this: the base row has to be poked into the Activation so that it is visible to the generation clauses when (2') runs. A similar poking is done for CHECK constraints. If you examine this poking for CHECK constraints, you will notice that sometimes the poking is undone after the constraints run and sometimes we don't bother to undo the poking. I don't understand the difference between these code paths. As a result, I have defensively coded the new poking which we need for generated columns. I poke the base row into the Activation just before the generation clauses run. After the generation clauses run, I return the Activation to its previous state. Here is a little more detail on the implementation: A) At bind() time we do the following: i) Prune out explicit mentions of generated columns. These can arise if the user sets a generated column to the literal DEFAULT--as allowed by the ANSI/ISO syntax. So for instance, the following is legal: insert into T( refCol, generatedCol ) values ( 1, default ) We prune out the explicitly added generated columns because, later on in the bind() phase, the insert list is expanded to include all columns with defaults (not just generated columns). ii) When the insert list is expanded to include all defaulted columns, we add in the generated columns but we don't bind their expressions. This is because the generation clause may refer to other columns in the base row. This, in turn, creates an ordering problem. In addition we we don't yet have a result set number for the base row--we need that number in order to bind references to other columns which may appear in the generation clauses. iii) Later on, just before we parse and bind the CHECK constraints, we parse and bind the generation clauses. At this point, we have enough context to bind the referenced columns. B) At generate() time, we generate method (2') in between generating (1') and (3'). The generated (2') method is now one of the arguments to the factory method which creates the execution-driver, the InsertResultSet. This is just like what we do for CHECK constraints: the generated (3') method is also an argument to the instantiation of the InsertResultSet. C) At execution time, we evaluate (2') just before we evaluate (3'). Touches the following files: -------------------- – BINDING -------------------- M java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java Adds a method so that a ResultColumn can report whether it represents a generated column. I also forced all overrides of the expression field to go through the setExpression() method. This, technically speaking, is not necessary--but it made debugging easier for me and I think it will be useful for other developers who need to debug this node. M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java Changes are made to support both binding and code-generation. These are the bind() changes: i) Adds a method to object if the user tries to override the value in a generated column with any value other than the DEFAULT literal. For instance, the following is illegal: insert into T( refCol, generatedCol ) values ( 1, 70 ) In addition, we remove explicit mentions of generated columns because we will add them back when we enhance the INSERT statement with defaulted columns. ii) Adds logic to parse and bind generated columns. This is modelled on the logic which parses and binds CHECK constraints. iii) Renames bindCheckConstraint() to bindRowScopedExpression() because this method is now shared by the logic which binds CHECK constraints and the logic which binds generation clauses. M java/engine/org/apache/derby/impl/sql/compile/ResultSetNode.java Short-circuits the logic which enhances the base row with defaulted columns. Adds in the generated columns but does not add their generation clauses. This is because the clauses cannot be bound at the same time as the rest of the columns in the base row. We wait to bind them until the time that we bind CHECK constraints. M java/engine/org/apache/derby/impl/sql/compile/InsertNode.java Wires binding and code-generation calls into bindStatement() and generate(). -------------------- – CODE GENERATION -------------------- M java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java Skips code-generation for generated columns when walking the base row. The generateCore() method generates (1'). We need to build the generation clauses into (2') instead and this is done later on. M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java In addition to the bind() changes described above, adds logic to generate the (2') method. -------------------- – EXECUTION -------------------- M java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java M java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java Adds (2') as an argument to the factory method which instantiates InsertResultSets. M java/engine/org/apache/derby/iapi/sql/Activation.java M java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java M java/engine/org/apache/derby/impl/sql/GenericActivationHolder.java Adds a method for retrieving the current row from the Activation. This allows us to return the Activation to its original state after we have run (2'). M java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java Evaluates generation clauses close to where CHECK constraints are evaluated. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Uncomments basic INSERT tests.
          Hide
          Rick Hillegas added a comment -

          Commited derby-03-aa-grammar.diff at subversion revision 708169.

          Show
          Rick Hillegas added a comment - Commited derby-03-aa-grammar.diff at subversion revision 708169.
          Hide
          Rick Hillegas added a comment -

          Hi Bryan,

          Thanks for reading the patches. I appreciate having another set of eyes on this!

          I think that your questions will be addressed in the next couple of patches, which should introduce INSERT-time and UPDATE-time processing. In coding INSERT/UPDATE support, I took CHECK constraints as my model. This is what's going on in the prototype:

          When you INSERT into a table which has a generated column:

          1) During compilation, at generate() time, we build a function to evaluate the generation clause.

          2) At execution time, we run that function on every row that will be inserted into the table. The function runs AFTER the non-generated columns are populated but BEFORE we run triggers, enforce constraints, and build index rows. All of this happens before the base and index rows are handed to the Store.

          When you UPDATE a table which has a generated column:

          3) During compilation, at bind() time we determine whether we need to re-evaluate any of the generation clauses. We decide to re-generate a column if we see that one of the columns that it references is being changed. So in the example you gave, we would decide to re-generate c given the following update statement:

          update bry set b = b + 3

          but we would decide NOT to re-generate c given the following update statement

          update bry set a = a + 3

          4) At execution time, we run that function on every row that is being updated. As with INSERT, the function runs AFTER the non-generated columns are populated but BEFORE we run triggers, enforce constraints, and build index rows.

          That, at least, is how the prototype is supposed to work! So to answer your questions directly:

          A) Generation clauses are evaluated when you INSERT and UPDATE rows.

          B) For UPDATEs, we re-evaluate a generation clause only if we are changing one of the columns that it references.

          Thanks,
          -Rick

          Show
          Rick Hillegas added a comment - Hi Bryan, Thanks for reading the patches. I appreciate having another set of eyes on this! I think that your questions will be addressed in the next couple of patches, which should introduce INSERT-time and UPDATE-time processing. In coding INSERT/UPDATE support, I took CHECK constraints as my model. This is what's going on in the prototype: When you INSERT into a table which has a generated column: 1) During compilation, at generate() time, we build a function to evaluate the generation clause. 2) At execution time, we run that function on every row that will be inserted into the table. The function runs AFTER the non-generated columns are populated but BEFORE we run triggers, enforce constraints, and build index rows. All of this happens before the base and index rows are handed to the Store. When you UPDATE a table which has a generated column: 3) During compilation, at bind() time we determine whether we need to re-evaluate any of the generation clauses. We decide to re-generate a column if we see that one of the columns that it references is being changed. So in the example you gave, we would decide to re-generate c given the following update statement: update bry set b = b + 3 but we would decide NOT to re-generate c given the following update statement update bry set a = a + 3 4) At execution time, we run that function on every row that is being updated. As with INSERT, the function runs AFTER the non-generated columns are populated but BEFORE we run triggers, enforce constraints, and build index rows. That, at least, is how the prototype is supposed to work! So to answer your questions directly: A) Generation clauses are evaluated when you INSERT and UPDATE rows. B) For UPDATEs, we re-evaluate a generation clause only if we are changing one of the columns that it references. Thanks, -Rick
          Hide
          Bryan Pendleton added a comment -

          Hi Rick,

          I read through the first 4 patches and they look very good to me. Thanks for the
          effort in writing the comments and the tests.

          I was wondering if you could briefly outline how you expect the execution-time
          logic to go: will we generate code to implement the generation expression?
          Or will there be some sort of interpreter that runs as part of update processing
          and interprets the generation expression?

          Also, when are the generated columns evaluated? Is it only when their value
          is retrieved by a SELECT? Or is the generated column evaluated whenever
          anything that it references is updated?

          So for example, if I have:
          create table bry (a int, b int, c int generated always as (2 * b))
          then is the value of column c generated:

          • only when c is selected?
          • when the value of b changes?
          • when the value of either a or b changes?

          thanks,

          bryan

          Show
          Bryan Pendleton added a comment - Hi Rick, I read through the first 4 patches and they look very good to me. Thanks for the effort in writing the comments and the tests. I was wondering if you could briefly outline how you expect the execution-time logic to go: will we generate code to implement the generation expression? Or will there be some sort of interpreter that runs as part of update processing and interprets the generation expression? Also, when are the generated columns evaluated? Is it only when their value is retrieved by a SELECT? Or is the generated column evaluated whenever anything that it references is updated? So for example, if I have: create table bry (a int, b int, c int generated always as (2 * b)) then is the value of column c generated: only when c is selected? when the value of b changes? when the value of either a or b changes? thanks, bryan
          Hide
          Rick Hillegas added a comment -

          Tests ran cleanly for me on the derby-481-03-aa-grammar.diff patch.

          Show
          Rick Hillegas added a comment - Tests ran cleanly for me on the derby-481-03-aa-grammar.diff patch.
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3923 [ DERBY-3923 ]
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3922 [ DERBY-3922 ]
          Rick Hillegas made changes -
          Attachment derby-481-03-aa-grammar.diff [ 12392801 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-03-aa-grammar.diff. I am running regression tests now.

          This patch does the following:

          1) Makes it possible to declare generated columns in CREATE TABLE statements.

          2) Adjusts the dblook tool to emit the correct DDL for tables with generated columns.

          3) Adds some basic syntax tests for CREATE TABLE statements which declare generated columns.

          4) Verifies that you can declare generated columns only after hard-upgrading to 10.5.

          I will file new JIRAs to track the following known deficiencies:

          A) You cannot add generated columns using ALTER TABLE yet. This behavior can not be tested until we check in the UPDATE logic for generated columns.

          B) The ANSI/ISO grammar lets you omit the column datatype on columns with generation clauses. The patch does not support this elegant shorthand.

          Touches the following files:

          ----------------
          – PARSING
          ----------------

          M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj

          Wires GENERATED ALWAYS AS (...) into the CREATE/ALTER TABLE grammar. This bit of grammar is known in the SQL spec as a "generation clause".

          M java/engine/org/apache/derby/impl/sql/compile/NodeFactoryImpl.java
          M java/engine/org/apache/derby/iapi/sql/compile/C_NodeTypes.java
          M java/engine/org/apache/derby/impl/sql/compile/C_NodeNames.java
          A java/engine/org/apache/derby/impl/sql/compile/GenerationClauseNode.java
          M java/engine/org/apache/derby/impl/sql/compile/ColumnDefinitionNode.java

          Adds a new kind of Abstract Syntax Tree (AST) node to represent generation clauses. In the AST, generation clauses decorate column definitions.

          ----------------
          – BINDING
          ----------------

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

          Logic to drive the bind() phase for generation clauses.

          M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java
          M java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java
          M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java
          M java/engine/org/apache/derby/impl/sql/compile/ValueNode.java

          Bind-time logic to prevent non-deterministic functions from appearing in generation clauses.

          M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java
          M java/engine/org/apache/derby/impl/sql/compile/DDLStatementNode.java
          M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java

          Factored some very similar bind-time logic out of the AST nodes for CREATE and ALTER TABLE. Consolidated this logic in the superclass of these two nodes.

          ----------------
          – TOOLS
          ----------------

          M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java
          M java/tools/org/apache/derby/impl/tools/dblook/DB_Table.java

          Code to make the dblook utility correctly emit generation clauses when printing CREATE TABLE statements.

          ----------------
          – TESTS
          ----------------

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

          Uncomments the basic grammar tests. Adds a test to verify that generation clauses are correctly stored in SYSCOLUMNS.COLUMNDEFAULT.

          M java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/Changes10_5.java

          Adds a test case to verify that you can declare generation clauses only after hard-upgrading to 10.5.

          M java/testing/org/apache/derbyTesting/functionTests/tests/tools/dblook_makeDB.sql
          M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/dblook_test_net.out
          M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/dblook_test_net_territory.out
          M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test_territory.out
          M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/dblook_test_net.out
          M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/dblook_test_net_territory.out
          M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test.out

          Beefs up the dblook test with some examples of CREATE TABLE statements which have generation clauses.

          Show
          Rick Hillegas added a comment - Attaching derby-481-03-aa-grammar.diff. I am running regression tests now. This patch does the following: 1) Makes it possible to declare generated columns in CREATE TABLE statements. 2) Adjusts the dblook tool to emit the correct DDL for tables with generated columns. 3) Adds some basic syntax tests for CREATE TABLE statements which declare generated columns. 4) Verifies that you can declare generated columns only after hard-upgrading to 10.5. I will file new JIRAs to track the following known deficiencies: A) You cannot add generated columns using ALTER TABLE yet. This behavior can not be tested until we check in the UPDATE logic for generated columns. B) The ANSI/ISO grammar lets you omit the column datatype on columns with generation clauses. The patch does not support this elegant shorthand. Touches the following files: ---------------- – PARSING ---------------- M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj Wires GENERATED ALWAYS AS (...) into the CREATE/ALTER TABLE grammar. This bit of grammar is known in the SQL spec as a "generation clause". M java/engine/org/apache/derby/impl/sql/compile/NodeFactoryImpl.java M java/engine/org/apache/derby/iapi/sql/compile/C_NodeTypes.java M java/engine/org/apache/derby/impl/sql/compile/C_NodeNames.java A java/engine/org/apache/derby/impl/sql/compile/GenerationClauseNode.java M java/engine/org/apache/derby/impl/sql/compile/ColumnDefinitionNode.java Adds a new kind of Abstract Syntax Tree (AST) node to represent generation clauses. In the AST, generation clauses decorate column definitions. ---------------- – BINDING ---------------- M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java Logic to drive the bind() phase for generation clauses. M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java M java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java M java/engine/org/apache/derby/impl/sql/compile/ValueNode.java Bind-time logic to prevent non-deterministic functions from appearing in generation clauses. M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java M java/engine/org/apache/derby/impl/sql/compile/DDLStatementNode.java M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java Factored some very similar bind-time logic out of the AST nodes for CREATE and ALTER TABLE. Consolidated this logic in the superclass of these two nodes. ---------------- – TOOLS ---------------- M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java M java/tools/org/apache/derby/impl/tools/dblook/DB_Table.java Code to make the dblook utility correctly emit generation clauses when printing CREATE TABLE statements. ---------------- – TESTS ---------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java Uncomments the basic grammar tests. Adds a test to verify that generation clauses are correctly stored in SYSCOLUMNS.COLUMNDEFAULT. M java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/Changes10_5.java Adds a test case to verify that you can declare generation clauses only after hard-upgrading to 10.5. M java/testing/org/apache/derbyTesting/functionTests/tests/tools/dblook_makeDB.sql M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/dblook_test_net.out M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/dblook_test_net_territory.out M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test_territory.out M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/dblook_test_net.out M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/dblook_test_net_territory.out M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test.out Beefs up the dblook test with some examples of CREATE TABLE statements which have generation clauses.
          Hide
          Rick Hillegas added a comment -

          Committed derby-481-02-aa-utilities.diff at subversion revision 707619.

          Show
          Rick Hillegas added a comment - Committed derby-481-02-aa-utilities.diff at subversion revision 707619.
          Rick Hillegas made changes -
          Attachment derby-481-02-aa-utilities.diff [ 12392760 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-02-aa-utilities.diff. This patch adds some small, miscellaneous support which will be turned on in later patches. Regression tests pass cleanly for me. Touches the following files:

          M java/engine/org/apache/derby/iapi/util/StringUtil.java

          Added utility routine to stringify an int array.

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

          New error messages related to generated columns.

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

          A number of new test cases. New test cases are commented out right now but will be uncommented as we check in the functionality that they verify.

          Show
          Rick Hillegas added a comment - Attaching derby-481-02-aa-utilities.diff. This patch adds some small, miscellaneous support which will be turned on in later patches. Regression tests pass cleanly for me. Touches the following files: M java/engine/org/apache/derby/iapi/util/StringUtil.java Added utility routine to stringify an int array. M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java New error messages related to generated columns. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java A number of new test cases. New test cases are commented out right now but will be uncommented as we check in the functionality that they verify.
          Hide
          Rick Hillegas added a comment -

          Committed derby-481-01-aa-catalog.diff at subversion revision 707414.

          Show
          Rick Hillegas added a comment - Committed derby-481-01-aa-catalog.diff at subversion revision 707414.
          Hide
          Bryan Pendleton added a comment -

          That makes sense. Thanks for the clarification, and thanks for working on this feature!

          Show
          Bryan Pendleton added a comment - That makes sense. Thanks for the clarification, and thanks for working on this feature!
          Hide
          Rick Hillegas added a comment -

          Hi Bryan,

          Thanks for your interest in this patch! In general, I do want to provide tests with patches. For the catalog changes, however, I need grammar changes before I can really test anything meaningful. I considered introducing the catalog changes along with the grammar patch but I thought it would be better to chunk the catalog piece in first in order to reduce the size of the grammar patch.

          Show
          Rick Hillegas added a comment - Hi Bryan, Thanks for your interest in this patch! In general, I do want to provide tests with patches. For the catalog changes, however, I need grammar changes before I can really test anything meaningful. I considered introducing the catalog changes along with the grammar patch but I thought it would be better to chunk the catalog piece in first in order to reduce the size of the grammar patch.
          Hide
          Bryan Pendleton added a comment -

          I think the incremental approach is great, but why not do the tests incrementally,
          at the same time?

          Show
          Bryan Pendleton added a comment - I think the incremental approach is great, but why not do the tests incrementally, at the same time?
          Rick Hillegas made changes -
          Derby Info [Patch Available]
          Rick Hillegas made changes -
          Attachment derby-481-01-aa-catalog.diff [ 12392672 ]
          Hide
          Rick Hillegas added a comment -

          Attaching derby-481-01-aa-catalog.diff, the first small patch cut out of the prototype. Presented for review. Tests pass cleanly for me. Tests for these catalog changes will appear in later patches.

          This patch provides the catalog support for generated columns. Catalog support turned out to be simpler than the functional spec imagined. There is no need to add another column to SYS.SYSCOLUMNS. Instead, we can re-use the existing COLUMNDEFAULT column to store the generation clause.

          Touches the following files:

          M java/engine/org/apache/derby/catalog/DefaultInfo.java
          M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java

          Special constructor for default descriptors which describe generated columns. A default descriptor can tell you whether it represents a generated column. If it does, it can also tell you what other columns the generation clause references.

          M java/engine/org/apache/derby/iapi/sql/dictionary/ColumnDescriptor.java

          Column descriptors can now tell you if they are bound to generation clauses.

          M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java

          The table descriptor can now return a list of all the columns which have generation clauses.

          Show
          Rick Hillegas added a comment - Attaching derby-481-01-aa-catalog.diff, the first small patch cut out of the prototype. Presented for review. Tests pass cleanly for me. Tests for these catalog changes will appear in later patches. This patch provides the catalog support for generated columns. Catalog support turned out to be simpler than the functional spec imagined. There is no need to add another column to SYS.SYSCOLUMNS. Instead, we can re-use the existing COLUMNDEFAULT column to store the generation clause. Touches the following files: M java/engine/org/apache/derby/catalog/DefaultInfo.java M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java Special constructor for default descriptors which describe generated columns. A default descriptor can tell you whether it represents a generated column. If it does, it can also tell you what other columns the generation clause references. M java/engine/org/apache/derby/iapi/sql/dictionary/ColumnDescriptor.java Column descriptors can now tell you if they are bound to generation clauses. M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java The table descriptor can now return a list of all the columns which have generation clauses.
          Rick Hillegas made changes -
          Assignee Rick Hillegas [ rhillegas ]
          Rick Hillegas made changes -
          Attachment derby-481-00-aa-prototype.diff [ 12392658 ]
          Hide
          Rick Hillegas added a comment -

          Attaching a prototype implementation of basic machinery for generated columns. I intend to split this up into smaller chunks that are easier to review. This patch provides basic support for declaring generated columns and for populating them at insert/update time.

          o Basic tests written for:

          • CREATE TABLE
          • Cooperation of INSERT/UPDATE with constraints, triggers, and indexes

          o Known not to work:

          • Omitting the column datatype when declaring a generation clause

          o Not tested at all:

          • Schema evolution and statement invalidation
          • Permissions
          • dblook
          • Upgrade

          Contents of the patch:

          UTILITIES, MESSAGES, TESTS

          M java/engine/org/apache/derby/iapi/util/StringUtil.java
          M java/engine/org/apache/derby/loc/messages.xml
          M java/shared/org/apache/derby/shared/common/reference/SQLState.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java

          CATALOG SUPPORT

          M java/engine/org/apache/derby/catalog/DefaultInfo.java
          M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/ColumnDescriptor.java

          DECLARING GENERATED COLUMNS

          M java/engine/org/apache/derby/impl/sql/compile/NodeFactoryImpl.java
          M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
          M java/engine/org/apache/derby/impl/sql/compile/DDLStatementNode.java
          M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java
          M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java
          M java/engine/org/apache/derby/impl/sql/compile/C_NodeNames.java
          M java/engine/org/apache/derby/impl/sql/compile/ColumnDefinitionNode.java
          M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java
          M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java
          A java/engine/org/apache/derby/impl/sql/compile/GenerationClauseNode.java
          M java/engine/org/apache/derby/iapi/sql/compile/C_NodeTypes.java
          M java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java

          INSERTING GENERATED COLUMNS

          M java/engine/org/apache/derby/impl/sql/compile/InsertNode.java
          M java/engine/org/apache/derby/impl/sql/compile/ResultSetNode.java
          M java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java
          M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java
          M java/engine/org/apache/derby/impl/sql/compile/ExpressionClassBuilder.java
          M java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
          M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java
          M java/engine/org/apache/derby/impl/sql/compile/ValueNode.java
          M java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java
          M java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java
          M java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java
          M java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java

          UPDATING GENERATED COLUMNS

          M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java
          M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java
          M java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java
          M java/engine/org/apache/derby/impl/sql/GenericActivationHolder.java
          M java/engine/org/apache/derby/iapi/sql/Activation.java

          Show
          Rick Hillegas added a comment - Attaching a prototype implementation of basic machinery for generated columns. I intend to split this up into smaller chunks that are easier to review. This patch provides basic support for declaring generated columns and for populating them at insert/update time. o Basic tests written for: CREATE TABLE Cooperation of INSERT/UPDATE with constraints, triggers, and indexes o Known not to work: Omitting the column datatype when declaring a generation clause o Not tested at all: Schema evolution and statement invalidation Permissions dblook Upgrade Contents of the patch: UTILITIES, MESSAGES, TESTS M java/engine/org/apache/derby/iapi/util/StringUtil.java M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GeneratedColumnsTest.java CATALOG SUPPORT M java/engine/org/apache/derby/catalog/DefaultInfo.java M java/engine/org/apache/derby/catalog/types/DefaultInfoImpl.java M java/engine/org/apache/derby/iapi/sql/dictionary/TableDescriptor.java M java/engine/org/apache/derby/iapi/sql/dictionary/ColumnDescriptor.java DECLARING GENERATED COLUMNS M java/engine/org/apache/derby/impl/sql/compile/NodeFactoryImpl.java M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj M java/engine/org/apache/derby/impl/sql/compile/DDLStatementNode.java M java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java M java/engine/org/apache/derby/impl/sql/compile/C_NodeNames.java M java/engine/org/apache/derby/impl/sql/compile/ColumnDefinitionNode.java M java/engine/org/apache/derby/impl/sql/compile/CreateTableNode.java M java/engine/org/apache/derby/impl/sql/compile/TableElementList.java A java/engine/org/apache/derby/impl/sql/compile/GenerationClauseNode.java M java/engine/org/apache/derby/iapi/sql/compile/C_NodeTypes.java M java/engine/org/apache/derby/iapi/sql/compile/CompilerContext.java INSERTING GENERATED COLUMNS M java/engine/org/apache/derby/impl/sql/compile/InsertNode.java M java/engine/org/apache/derby/impl/sql/compile/ResultSetNode.java M java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java M java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java M java/engine/org/apache/derby/impl/sql/compile/ExpressionClassBuilder.java M java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java M java/engine/org/apache/derby/impl/sql/compile/ValueNode.java M java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java M java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java M java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java M java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java UPDATING GENERATED COLUMNS M java/engine/org/apache/derby/impl/sql/compile/UpdateNode.java M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java M java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java M java/engine/org/apache/derby/impl/sql/GenericActivationHolder.java M java/engine/org/apache/derby/iapi/sql/Activation.java
          Rick Hillegas made changes -
          Link This issue relates to DERBY-1748 [ DERBY-1748 ]
          Rick Hillegas made changes -
          Link This issue is related to DERBY-455 [ DERBY-455 ]
          Hide
          Rick Hillegas added a comment -

          Sounds like a good idea. We'll need to divide this work into subtasks as we go along. I've created a separate subtask (DERBY-3570) to track the DETERMINISTIC work.

          Show
          Rick Hillegas added a comment - Sounds like a good idea. We'll need to divide this work into subtasks as we go along. I've created a separate subtask ( DERBY-3570 ) to track the DETERMINISTIC work.
          Rick Hillegas made changes -
          Link This issue incorporates DERBY-3570 [ DERBY-3570 ]
          Hide
          Daniel John Debrunner added a comment -

          Since DeterministicCharacteristic is an independent feature to generated columns should it be its own Jira issue.
          If not we may land in a similar situation to DERBY-2109 where independent functionality is implemented but no record exists in Jira.

          Show
          Daniel John Debrunner added a comment - Since DeterministicCharacteristic is an independent feature to generated columns should it be its own Jira issue. If not we may land in a similar situation to DERBY-2109 where independent functionality is implemented but no record exists in Jira.
          Hide
          Rick Hillegas added a comment - - edited

          This feature was introduced to the SQL Standard in 2003. There it is called T175. Here is some information on what other databases do:

          DB2 - Appears to implement the full language for T175.

          Oracle - Does not appear to implement T175. However, some similar functionality is available. "Although Oracle does not support generated columns, a function-based index can be used to index on the result of an expression" according to http://download.oracle.com/docs/cd/B19306_01/server.102/b14200/ap_standard_sql004.htm

          Postgres - Does not appear to implement T175. However, according to section 5.2 of the Postgres 8.3 reference manual, it appears that you can declare the DEFAULT value of a column to be computed by an expression which can contain functions. In addition, according to section 11.7, Postgres lets you define indexes on expressions made up of functions and columns in the base row.

          MySQL - According to section 12.1.10 of the MySQL 6.0 reference manual, MySQL only allows constants and CURRENT_TIMESTAMP as default values for columns.

          Show
          Rick Hillegas added a comment - - edited This feature was introduced to the SQL Standard in 2003. There it is called T175. Here is some information on what other databases do: DB2 - Appears to implement the full language for T175. Oracle - Does not appear to implement T175. However, some similar functionality is available. "Although Oracle does not support generated columns, a function-based index can be used to index on the result of an expression" according to http://download.oracle.com/docs/cd/B19306_01/server.102/b14200/ap_standard_sql004.htm Postgres - Does not appear to implement T175. However, according to section 5.2 of the Postgres 8.3 reference manual, it appears that you can declare the DEFAULT value of a column to be computed by an expression which can contain functions. In addition, according to section 11.7, Postgres lets you define indexes on expressions made up of functions and columns in the base row. MySQL - According to section 12.1.10 of the MySQL 6.0 reference manual, MySQL only allows constants and CURRENT_TIMESTAMP as default values for columns.
          Hide
          Rick Hillegas added a comment -

          Thanks, Dan. That would be great.

          Show
          Rick Hillegas added a comment - Thanks, Dan. That would be great.
          Hide
          Daniel John Debrunner added a comment -

          In the spec:
          > We rename the sqlAllowed field of RoutineAliasInfo and borrow one of its bits to encode whether a routine is DETERMINISTIC.

          RoutineAliasInfo already has a field set up for expansion, I think we can use this in a smart way instead of making a single field handle multiple meanings (less clear). I've always been planning to add all the possible future options for routines into RoutineAliasInfo, I'd be willing to handle that portion for this improvement.

          Show
          Daniel John Debrunner added a comment - In the spec: > We rename the sqlAllowed field of RoutineAliasInfo and borrow one of its bits to encode whether a routine is DETERMINISTIC. RoutineAliasInfo already has a field set up for expansion, I think we can use this in a smart way instead of making a single field handle multiple meanings (less clear). I've always been planning to add all the possible future options for routines into RoutineAliasInfo, I'd be willing to handle that portion for this improvement.
          Rick Hillegas made changes -
          Field Original Value New Value
          Attachment GeneratedColumns.html [ 12378246 ]
          Hide
          Rick Hillegas added a comment - - edited

          Attaching a first rev of a functional spec for this feature: GeneratedColumns.html

          Show
          Rick Hillegas added a comment - - edited Attaching a first rev of a functional spec for this feature: GeneratedColumns.html
          Rick Hillegas created issue -

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development