Proposed ConstantAction split

[Daniel John Debrunner <dj...@apache.org>]

org.apache.derby.iapi.sql.execute.ConstantAction

I believe the ConstantAction is a single interface trying to represent
two concepts and would be better served being split.

ConstantAction has two methods, executeConstantAction() and
modifiesTableId(). You can ignore the upToDate() method as it is a no-op
and I'm removing it. It's a no-op because all the implementations return
true.

ConstantAction has two implementation sub-classes, each with their own
tree of implematation sub-classes, GenericConstantAction and
WriteCursorConstantAction.

org.apache.derby.impl.sql.execute.GenericConstantAction
  - has valid executeConstantAction() methods
  - has various implementations of modifiesTableId but they are never
called.

org.apache.derby.impl.sql.execute.WriteCursorConstantAction
  - has an empty executeConstantAction() method that is never called.
  - has a valid modifiesTableId method.


The method modifiesTableId is only called once in
InternalTriggerExecutionContext and then only if the constant action is
an instanceof WriteCursorConstantAction.

[aside - the if statement in InternalTriggerExecutionContext is very
dubious,  the else on line 341 matches the sanity check which seems
wrong. I'll look at the Cloudscape history to see if there is some
missing code here. Also there is a SQLState.LANG_NO_DDL_IN_TRIGGER state
that is never used, I wonder if it is meant to be used here.
end-aside]

So it seems there are two separate uses of ConstantAction.

1) An action that represents a SQL statement that does not require
generated code. This is what the name implies to me. These are executed
by the open method of either MiscResultSet or DDLResultSet.

2) A holder for execution constants for DML statements.

I don't see any overlap in these two uses, thus I think that the
hierarchy should be split.

Comments, questions?
Dan.

Re: Proposed ConstantAction split

[Rick Hillegas <Ri...@Sun.COM>]

This sounds reasonable to me. The interface is oddly overloaded. I think 
that ConstantActions began as a way to code readable execution code for 
DDL, avoiding unnecessary and hard-to-debug generated byte code. I 
notice that WriteCursorConstantAction implements Formatable. This 
suggests to me that, early on, it was part of the Synchronization logic, 
that is, it helped serialize replicated dml.

Regards,
-Rick

Daniel John Debrunner wrote:

>org.apache.derby.iapi.sql.execute.ConstantAction
>
>I believe the ConstantAction is a single interface trying to represent
>two concepts and would be better served being split.
>
>ConstantAction has two methods, executeConstantAction() and
>modifiesTableId(). You can ignore the upToDate() method as it is a no-op
>and I'm removing it. It's a no-op because all the implementations return
>true.
>
>ConstantAction has two implementation sub-classes, each with their own
>tree of implematation sub-classes, GenericConstantAction and
>WriteCursorConstantAction.
>
>org.apache.derby.impl.sql.execute.GenericConstantAction
>  - has valid executeConstantAction() methods
>  - has various implementations of modifiesTableId but they are never
>called.
>
>org.apache.derby.impl.sql.execute.WriteCursorConstantAction
>  - has an empty executeConstantAction() method that is never called.
>  - has a valid modifiesTableId method.
>
>
>The method modifiesTableId is only called once in
>InternalTriggerExecutionContext and then only if the constant action is
>an instanceof WriteCursorConstantAction.
>
>[aside - the if statement in InternalTriggerExecutionContext is very
>dubious,  the else on line 341 matches the sanity check which seems
>wrong. I'll look at the Cloudscape history to see if there is some
>missing code here. Also there is a SQLState.LANG_NO_DDL_IN_TRIGGER state
>that is never used, I wonder if it is meant to be used here.
>end-aside]
>
>So it seems there are two separate uses of ConstantAction.
>
>1) An action that represents a SQL statement that does not require
>generated code. This is what the name implies to me. These are executed
>by the open method of either MiscResultSet or DDLResultSet.
>
>2) A holder for execution constants for DML statements.
>
>I don't see any overlap in these two uses, thus I think that the
>hierarchy should be split.
>
>Comments, questions?
>Dan.
>
>
>
>  
>