[
https://issues.apache.org/jira/browse/DERBY-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14078636#comment-14078636
]
Dag H. Wanvik edited comment on DERBY-6670 at 7/30/14 5:57 PM:
---------------------------------------------------------------
Attaching version 2 of the augmented "ae" patch, [^derby-6670-2-b.diff]. This
time I also did a full review of that patch. I believe the refactoring to UUIDs
instead of conglomerate ids is sound and as far as I can see, complete. Thanks
for the refactoring, Knut and Rick, it makes the code simpler and paves the way
for the fix of this issue.
Details for this patch:
Review of the "ae" patch and details of changes relative to that one.
{code}
modified:
java/engine/org/apache/derby/iapi/sql/conn/LanguageConnectionContext.java
modified:
java/engine/org/apache/derby/impl/sql/conn/GenericLanguageConnectionContext.java
{code}
Removed forget*. The rest looks OK, change to UUIDs only.
{code}
modified:
java/engine/org/apache/derby/iapi/sql/conn/SQLSessionContext.java
modified:
java/engine/org/apache/derby/impl/sql/conn/SQLSessionContextImpl.java
{code}
Renamed the merged methods here so as not to imply they only work for check
constraints: they now work for
all constraints (there are no longer two variants of the methods).
{code}
modified:
java/engine/org/apache/derby/impl/sql/compile/ProjectRestrictNode.java
modified: java/engine/org/apache/derby/catalog/UUID.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ProjectRestrictResultSet.java
modified:
java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java
modified:
java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java
modified: java/engine/org/apache/derby/impl/sql/execute/FKInfo.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ReferencedKeyRIChecker.java
modified:
java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java
modified:
java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ConstraintConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/CreateConstraintConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/CreateIndexConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ForeignKeyRIChecker.java
modified:
java/engine/org/apache/derby/impl/sql/execute/RIBulkChecker.java
modified:
java/engine/org/apache/derby/impl/sql/execute/SetConstraintsConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/UniqueIndexSortObserver.java
modified:
java/engine/org/apache/derby/impl/sql/execute/UniqueWithDuplicateNullsIndexSortObserver.java
modified:
java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ValidateCheckConstraintResultSet.java
modified:
java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java
{code}
OK; move to UUIDs only.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/IndexConstantAction.java
{code}
OK; move to UUIDs only. But why "transient"?
{code}
modified:
java/engine/org/apache/derby/iapi/sql/dictionary/ConstraintDescriptor.java
{code}
Removed forgetDeferredConstraintsData calls. Solves DERBY-6670.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/DeferredConstraintsMemory.java
{code}
Made sure we always bail out (all is good) iff dictionary objects do
not exist when we try to check violations. Solves DERBY-6670.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/IndexChanger.java
{code}
Removed the TODO: mapping back to constraint id is unique (one-to-one) since we
do not share indexes
for deferrable constraints.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java
{code}
OK; move to UUIDs only, but simplified some unncessary conditionals.
{code}
modified:
java/testing/org/apache/derbyTesting/functionTests/tests/lang/ConstraintCharacteristicsTest.java
{code}
Added 6670 repro + more rollback to savepoint scenarios.
Running regressions. {color:green}[Update: regressions passed ok]{color}
was (Author: dagw):
Attaching version 2 of the augmented "ae" patch, [^derby-6670-2-b.diff]. This
time I also did a full review of that patch. I believe the refactoring to UUIDs
instead of conglomerate ids is sound and as far as I can see, complete. Thanks
for the refactoring, Knut and Rick, it makes the code simpler and paves the way
for the fix of this issue.
Details for this patch:
Review of the "ae" patch and details of changes relative to that one.
{code}
modified:
java/engine/org/apache/derby/iapi/sql/conn/LanguageConnectionContext.java
modified:
java/engine/org/apache/derby/impl/sql/conn/GenericLanguageConnectionContext.java
{code}
Removed forget*. The rest looks OK, change to UUIDs only.
{code}
modified:
java/engine/org/apache/derby/iapi/sql/conn/SQLSessionContext.java
modified:
java/engine/org/apache/derby/impl/sql/conn/SQLSessionContextImpl.java
{code}
Renamed the merged methods here so as not to imply they only work for check
constraints: they now work for
all constraints (there are no longer two variants of the methods).
{code}
modified:
java/engine/org/apache/derby/impl/sql/compile/ProjectRestrictNode.java
modified: java/engine/org/apache/derby/catalog/UUID.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ProjectRestrictResultSet.java
modified:
java/engine/org/apache/derby/iapi/sql/execute/ResultSetFactory.java
modified:
java/engine/org/apache/derby/impl/sql/compile/DMLModStatementNode.java
modified: java/engine/org/apache/derby/impl/sql/execute/FKInfo.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ReferencedKeyRIChecker.java
modified:
java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java
modified:
java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ConstraintConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/CreateConstraintConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/CreateIndexConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ForeignKeyRIChecker.java
modified:
java/engine/org/apache/derby/impl/sql/execute/RIBulkChecker.java
modified:
java/engine/org/apache/derby/impl/sql/execute/SetConstraintsConstantAction.java
modified:
java/engine/org/apache/derby/impl/sql/execute/UniqueIndexSortObserver.java
modified:
java/engine/org/apache/derby/impl/sql/execute/UniqueWithDuplicateNullsIndexSortObserver.java
modified:
java/engine/org/apache/derby/impl/sql/execute/UpdateResultSet.java
modified:
java/engine/org/apache/derby/impl/sql/execute/ValidateCheckConstraintResultSet.java
modified:
java/engine/org/apache/derby/impl/sql/execute/GenericResultSetFactory.java
{code}
OK; move to UUIDs only.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/IndexConstantAction.java
{code}
OK; move to UUIDs only. But why "transient"?
{code}
modified:
java/engine/org/apache/derby/iapi/sql/dictionary/ConstraintDescriptor.java
{code}
Removed forgetDeferredConstraintsData calls. Solves DERBY-6670.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/DeferredConstraintsMemory.java
{code}
Made sure we always bail out (all is good) iff dictionary objects do
not exist when we try to check violations. Solves DERBY-6670.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/IndexChanger.java
{code}
Removed the TODO: mapping back to constraint id is unique (one-to-one) since we
do not share indexes
for deferrable constraints.
{code}
modified:
java/engine/org/apache/derby/impl/sql/execute/InsertResultSet.java
{code}
OK; move to UUIDs only, but simplified some unncessary conditionals.
{code}
modified:
java/testing/org/apache/derbyTesting/functionTests/tests/lang/ConstraintCharacteristicsTest.java
{code}
Added 6670 repro + more rollback to savepoint scenarios.
Running regressions.
> Rollback to savepoint allows violation of deferrable constraints
> ----------------------------------------------------------------
>
> Key: DERBY-6670
> URL: https://issues.apache.org/jira/browse/DERBY-6670
> Project: Derby
> Issue Type: Bug
> Components: SQL
> Affects Versions: 10.11.0.0
> Reporter: Knut Anders Hatlen
> Attachments:
> derby-6670-01-aa-forbidSavepointsWithDeferredConstraints.diff,
> derby-6670-2-a.diff, derby-6670-2-a.status, derby-6670-2-b.diff,
> derby-6670-2-b.status
>
>
> The bug is illustrated by the following code snippet:
> {code}
> Connection c =
> DriverManager.getConnection("jdbc:derby:memory:db;create=true");
> c.setAutoCommit(false);
> Statement s = c.createStatement();
> s.execute("create table t1(x int primary key initially deferred)");
> s.execute("insert into t1 values 1,1,1,1");
> Savepoint sp = c.setSavepoint();
> s.execute("drop table t1");
> c.rollback(sp);
> // Since there are four identical rows in T1, this call should have
> // failed because the primary key was violated.
> c.commit();
> // Instead, it succeeds, and all four rows are committed, as can
> // be seen here:
> ResultSet rs = s.executeQuery("select * from t1");
> while (rs.next()) {
> System.out.println(rs.getInt(1));
> }
> // Insert yet another row, so that we have five identical rows ...
> s.execute("insert into t1 values 1");
> // ... and now commit complains ...
> c.commit();
> {code}
> With auto-commit off, add duplicates into a deferred primary key. Then set a
> savepoint, drop the table, and roll back to the savepoint.
> Apparently, when you drop the table, information about any constraint
> violations seen on that table is lost, and that information is not restored
> when the drop table operation is undone by the rollback to savepoint.
> So when you commit the transaction after having rolled back the drop
> operation, no deferred checking of constraints happens, and the duplicates
> you have inserted are committed.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
