Hi Michael,
Thanks for reviewing my fix! :-)
I ran tests against Oracle and checked in the fix.
Regards,
-Wonseok
On 4/11/07, Michael Bouschen <Michael.Bouschen_at_sun.com> wrote:
>
> Hi Wonseok,
>
> your fix looks good. From my point of view it is ready to be checked in.
> Do you have a chance to run the entity-persistence-tests with your
> changes against oracle? If not, I can do this.
>
> Regards Michael
>
>
> > Tom, Michael
> >
> > Please review the below fix for issue 2799
> > https://glassfish.dev.java.net/issues/show_bug.cgi?id=2799
> >
> > * Investigation:
> >
> > ParseTree.initBaseExpression() has problem when setting base
> > expression for base variable. For Update query which has not
> > qualfication variable, lower-case(canonicalized) abstrat schema name
> > is used for base variable like "employee", but initBaseExpression() is
> > using abstract schema name itself like "Employee" without
> > canonicalizing and put "Employee" -> ExpressionBuilder to map as base
> > expression. This caused generating a little different(invalid)
> > expression for unqualified where node.
> >
> > The invalid base expression(of generated QueryKeyExpression) affects
> > generating invalid SQL for MySQL which always uses temporary table for
> > bulk update.
> >
> > * Fix:
> > Extract the method of getting canonical base variable name in
> > ModifyQuery.validate() and reuse the method in
> > ParseTree.initBaseExpression().
> >
> > * Diff:
> >
> > Index:
> >
> entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ModifyNode.java
> > ===================================================================
> > RCS file:
> >
> /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ModifyNode.java,v
> > retrieving revision 1.6
> > diff -c -w -r1.6 ModifyNode.java
> > ***
> >
> entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ModifyNode.java
>
> > 4 Jan 2007 14:31:31 -0000 1.6
> > ---
> >
> entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ModifyNode.java
> > 10 Apr 2007 09:09:02 -0000
> > ***************
> > *** 49,57 ****
> > public void validate(ParseTreeContext context) {
> > // If defined use the abstractSchemaIdentifier as the base
> > variable,
> > // otherwise use the abstractSchemaName
> > ! String baseVariable = abstractSchemaIdentifier != null ?
> > ! abstractSchemaIdentifier : abstractSchemaName;
> > ! baseVariable =
> > IdentificationVariableDeclNode.calculateCanonicalName(baseVariable);
> > context.setBaseVariable (baseVariable);
> > super.validate(context);
> > }
> > --- 49,55 ----
> > public void validate(ParseTreeContext context) {
> > // If defined use the abstractSchemaIdentifier as the base
> > variable,
> > // otherwise use the abstractSchemaName
> > ! String baseVariable = getCanonicalAbstractSchemaIdentifier();
> > context.setBaseVariable(baseVariable);
> > super.validate(context);
> > }
> > ***************
> > *** 92,97 ****
> > --- 90,107 ----
> > }
> >
> > /**
> > + * INTERNAL:
> > + * Returns the canonical name of abstract schema identifier.
> > + * If the identifier is not specified(unqualified attribute
> > scenario),
> > + * the canonical name of abstract schema is returned.
> > + */
> > + public String getCanonicalAbstractSchemaIdentifier() {
> > + String variable = abstractSchemaIdentifier != null ?
> > + abstractSchemaIdentifier : abstractSchemaName;
> > + return
> > IdentificationVariableDeclNode.calculateCanonicalName (variable);
> > + }
> > +
> > + /**
> > * resolveClass: Answer the class which corresponds to my
> > variableName. This is the class for
> > * an alias, where the variableName is registered to an alias.
> > */
> > Index:
> >
> entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ParseTree.java
> >
> > ===================================================================
> > RCS file:
> >
> /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ParseTree.java,v
> > retrieving revision 1.25
> > diff -c -w - r1.25 ParseTree.java
> > ***
> >
> entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ParseTree.java
>
> > 4 Jan 2007 14:31:33 -0000 1.25
> > ---
> >
> entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/ParseTree.java
> > 10 Apr 2007 09:09:02 -0000
> > ***************
> > *** 139,151 ****
> > */
> > public void initBaseExpression(ModifyAllQuery theQuery,
> > GenerationContext generationContext) {
> > ModifyNode queryNode = (ModifyNode)getQueryNode();
> > ! String variable = queryNode.getAbstractSchemaIdentifier();
> > ! if (variable == null) {
> > ! // UPDATE and DELETE may not define a variable =>
> > ! // use schema name as variable
> > ! variable = queryNode.getAbstractSchemaName();
> > ! }
> > ! ParseTreeContext context =
> > generationContext.getParseTreeContext();
> > Class referenceClass = theQuery.getReferenceClass ();
> > // Create a new expression builder for the reference class
> > ExpressionBuilder builder = new
> > ExpressionBuilder(referenceClass);
> > --- 139,145 ----
> > */
> > public void initBaseExpression(ModifyAllQuery theQuery,
> > GenerationContext generationContext) {
> > ModifyNode queryNode = (ModifyNode)getQueryNode();
> > ! String variable =
> > queryNode.getCanonicalAbstractSchemaIdentifier();
> > Class referenceClass = theQuery.getReferenceClass();
> > // Create a new expression builder for the reference class
> > ExpressionBuilder builder = new
> > ExpressionBuilder(referenceClass);
> >
> > NOTE: I also removed the line "ParseTreeContext context =
> > generationContext.getParseTreeContext();" which is not referenced
> > anywhere.
> >
> > * Tests:
> > I ran entity-persistence-tests on MySQL and Derby.
> > Finally, all entity-persistence-tests has passed without any failure
> > on MySQL InnoDB engine!
> >
> > Thanks,
> > -Wonseok
>