Merge pull request #4616 from ghalliday/issue9660

HPCC-9660 Fix optimization of a DENORMALIZE with an empty rhs

Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/hql/hqlfold.cpp

@@ -3449,8 +3449,14 @@ IHqlExpression * NullFolderMixin::foldNullDataset(IHqlExpression * expr)
                     cvtRightProject = true;
                 else if (rightIsNull)
                 {
+                    //JOIN(ds,<null>) becomes a project
+                    //DENORMALIZE(ds, <null>) becomes a nop (since the transform will not be called)
+                    //DENORMALIZE(ds, <null>, GROUP) becomes a project
+                    if (op == no_denormalize)
+                        return removeParentNode(expr);  // ok because this returns queryChild(0)
+
                     cvtLeftProject = true;
-                    reason = "JOIN(ds,<empty>)";
+                    reason = "(ds,<empty>)";
                 }
             }
 
@@ -3461,8 +3467,11 @@ IHqlExpression * NullFolderMixin::foldNullDataset(IHqlExpression * expr)
                 //Never matches, so either LHS is modified by the transform - like a project, or it never returns anything.
                 if (isLeftJoin(expr))
                 {
+                    if (op == no_denormalize)
+                        return removeParentNode(expr);  // ok because this returns queryChild(0)
+
                     cvtLeftProject = true;
-                    reason = "JOIN(false)";
+                    reason = "(false)";
                 }
                 else if (isInnerJoin(expr))
                     return replaceWithNull(expr);
@@ -3476,11 +3485,11 @@ IHqlExpression * NullFolderMixin::foldNullDataset(IHqlExpression * expr)
                 if (isSpecificJoin(expr, leftouterAtom))
                 {
                     if (matchesConstantValue(queryPropertyChild(expr, keepAtom, 0), 1))
-                        potentialLeftProjectReason = "JOIN(,LEFT OUTER,KEEP(1))";
+                        potentialLeftProjectReason = "(,LEFT OUTER,KEEP(1))";
                     else if (expr->hasProperty(lookupAtom) && !expr->hasProperty(manyAtom))
-                        potentialLeftProjectReason = "JOIN(,LEFT OUTER,SINGLE LOOKUP)";
+                        potentialLeftProjectReason = "(,LEFT OUTER,SINGLE LOOKUP)";
                     else if (hasNoMoreRowsThan(expr, 1))
-                        potentialLeftProjectReason = "JOIN(<single-row>,LEFT OUTER)";
+                        potentialLeftProjectReason = "(<single-row>,LEFT OUTER)";
                 }
 
                 if (potentialLeftProjectReason)
@@ -3497,8 +3506,10 @@ IHqlExpression * NullFolderMixin::foldNullDataset(IHqlExpression * expr)
 
                     if (cvtLeftProject && (expr->getOperator() == no_denormalize))
                     {
+                        OwnedHqlExpr left = createSelector(no_left, child, selSeq);
                         //Denormalize with no match will not call the transform, so we can't convert that to a project
-                        if (containsSkip(transform))
+                        //unless the transform is a nop
+                        if (!transformReturnsSide(expr, no_left, 0))
                             cvtLeftProject = false;
                     }
                 }
@@ -3517,12 +3528,22 @@ IHqlExpression * NullFolderMixin::foldNullDataset(IHqlExpression * expr)
                     OwnedHqlExpr nullExpr = createDataset(no_null, LINK(rhs->queryRecord()));
                     newTransform.setown(replaceExpression(newTransform, rowsExpr, nullExpr));
                 }
+                if (op == no_denormalize)
+                {
+                    IHqlExpression * counter = queryPropertyChild(expr, _countProject_Atom, 0);
+                    if (counter)
+                    {
+                        OwnedHqlExpr one = createConstant(counter->queryType()->castFrom(false, I64C(1)));
+                        //Remove the annotations from the transform, otherwise it may say t(LEFT,COUNTER) which is confusing.
+                        newTransform.setown(replaceExpression(newTransform->queryBody(), counter, one));
+                    }
+                }
                 HqlExprArray args;
                 args.append(*preserveGrouping(child, expr));
                 args.append(*newTransform.getClear());
                 args.append(*LINK(selSeq));
                 OwnedHqlExpr ret = createDataset(no_hqlproject, args);
-                DBGLOG("Folder: Replace %s with PROJECT", reason);
+                DBGLOG("Folder: Replace %s%s with PROJECT", getOpString(op), reason);
                 return ret.getClear();
             }
 

ecl/regress/issue9660.ecl

@@ -0,0 +1,49 @@
+/*##############################################################################
+
+    HPCC SYSTEMS software Copyright (C) 2012 HPCC Systems.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+############################################################################## */
+
+namesRecord :=
+            RECORD
+string20        surname;
+string10        forename;
+integer2        age := 25;
+            END;
+
+namesTable := dataset([
+        {'Hawthorn','Gavin',31},
+        {'Hawthorn','Mia',30},
+        {'Smithe','Pru',10},
+        {'X','Z'}], namesRecord);
+
+namesTable2 := dataset([
+        {'Hawthorn','Gavin',1},
+        {'Hawthorn','Mia',2},
+        {'Smithe','Pru',3},
+        {'X','Z'}], namesRecord);
+
+namesRecord t(namesRecord l) := TRANSFORM
+    SELF.age := l.age + 1;
+    SELF := l;
+END;
+
+null1 := NOFOLD(namesTable2)(age > 100);
+o1 := output(DENORMALIZE(namesTable, null1, LEFT.surname = RIGHT.surname, t(LEFT)));
+
+null2 := namesTable2(false);
+o2 := output(DENORMALIZE(namesTable, null2, LEFT.surname = RIGHT.surname, t(LEFT)));
+
+
+SEQUENTIAL(o1, o2);