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