Przeglądaj źródła

Fix incorrect duplicated code

Some of the transforms were checking whether an expression had already
been visited, instead of checking whether the body had been visited.
This could lead to children being counted more than once, and
potentially duplicating branches.

Has a significant effect on a small number of archived queries, and a
minor effect on several others.

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 13 lat temu
rodzic
commit
c92fa6f921

+ 2 - 2
ecl/hql/hqltrans.cpp

@@ -2163,8 +2163,8 @@ IHqlExpression * HoistingHqlTransformer::transformRoot(IHqlExpression * expr)
 
 
 bool HoistingHqlTransformer::analyseThis(IHqlExpression * expr)
 bool HoistingHqlTransformer::analyseThis(IHqlExpression * expr)
 {
 {
-    HoistingTransformInfo * extra = queryExtra(expr->queryBody());
-    if (alreadyVisited(expr))
+    HoistingTransformInfo * extra = queryBodyExtra(expr);
+    if (alreadyVisited(expr->queryBody()))
     {
     {
         if ((pass != 0) || extra->isUnconditional() || (conditionDepth > 0))
         if ((pass != 0) || extra->isUnconditional() || (conditionDepth > 0))
             return false;
             return false;

+ 2 - 2
ecl/hql/hqltrans.ipp

@@ -622,8 +622,8 @@ protected:
     virtual void doAnalyseExpr(IHqlExpression * expr);
     virtual void doAnalyseExpr(IHqlExpression * expr);
 
 
     virtual ANewTransformInfo * createTransformInfo(IHqlExpression * expr);
     virtual ANewTransformInfo * createTransformInfo(IHqlExpression * expr);
-    inline HoistingTransformInfo * queryExtra(IHqlExpression * expr)        { return static_cast<HoistingTransformInfo *>(queryTransformExtra(expr)); }
-    inline bool isUsedUnconditionally(IHqlExpression * expr)                { return queryExtra(expr->queryBody())->isUnconditional(); }
+    inline HoistingTransformInfo * queryBodyExtra(IHqlExpression * expr)        { return static_cast<HoistingTransformInfo *>(queryTransformExtra(expr->queryBody())); }
+    inline bool isUsedUnconditionally(IHqlExpression * expr)                { return queryBodyExtra(expr)->isUnconditional(); }
 
 
     virtual void analyseExpr(IHqlExpression * expr);
     virtual void analyseExpr(IHqlExpression * expr);
     virtual IHqlExpression * createTransformed(IHqlExpression * expr);
     virtual IHqlExpression * createTransformed(IHqlExpression * expr);

+ 4 - 4
ecl/hqlcpp/hqlhtcpp.cpp

@@ -364,7 +364,7 @@ public:
     {
     {
         return CREATE_NEWTRANSFORMINFO(ChildSpotterInfo, expr);
         return CREATE_NEWTRANSFORMINFO(ChildSpotterInfo, expr);
     }
     }
-    inline ChildSpotterInfo * queryExtra(IHqlExpression * expr)     { return static_cast<ChildSpotterInfo *>(queryTransformExtra(expr)); }
+    inline ChildSpotterInfo * queryBodyExtra(IHqlExpression * expr)     { return static_cast<ChildSpotterInfo *>(queryTransformExtra(expr->queryBody())); }
 
 
     virtual void analyseExpr(IHqlExpression * expr)
     virtual void analyseExpr(IHqlExpression * expr)
     {
     {
@@ -406,7 +406,7 @@ public:
         }
         }
         else
         else
         {
         {
-            if (!alreadyVisited(expr))
+            if (!alreadyVisited(expr->queryBody()))
                 markHoistPoints(expr);
                 markHoistPoints(expr);
         }
         }
     }
     }
@@ -453,7 +453,7 @@ public:
             if (isUsedUnconditionallyEnough(expr) && !translator.canAssignInline(&ctx, expr))
             if (isUsedUnconditionallyEnough(expr) && !translator.canAssignInline(&ctx, expr))
             {
             {
                 candidate = true;
                 candidate = true;
-                queryExtra(expr)->setHoist();
+                queryBodyExtra(expr)->setHoist();
                 return;
                 return;
             }
             }
             if (walkFurtherDownTree(expr))
             if (walkFurtherDownTree(expr))
@@ -608,7 +608,7 @@ public:
 
 
         if ((expr->isDataset() || expr->isDatarow()) && (expr->getOperator() != no_select))
         if ((expr->isDataset() || expr->isDatarow()) && (expr->getOperator() != no_select))
         {
         {
-            if (queryExtra(expr)->getHoist() && !translator.canAssignInline(&ctx, transformed))
+            if (queryBodyExtra(expr)->getHoist() && !translator.canAssignInline(&ctx, transformed))
             {
             {
                 if (!builder)
                 if (!builder)
                     builder.setown(new ChildGraphBuilder(translator));
                     builder.setown(new ChildGraphBuilder(translator));

+ 1 - 1
ecl/hqlcpp/hqlttcpp.cpp

@@ -4000,7 +4000,7 @@ void CompoundSourceTransformer::analyseMarkBoundaries(IHqlExpression * expr)
 
 
 void CompoundSourceTransformer::analyseExpr(IHqlExpression * expr)
 void CompoundSourceTransformer::analyseExpr(IHqlExpression * expr)
 {
 {
-    if (alreadyVisited(expr))
+    if (alreadyVisited(expr->queryBody()))
     {
     {
         if ((pass == 0) && !insideCompound)
         if ((pass == 0) && !insideCompound)
         {
         {