فهرست منبع

HPCC-15565 Fix problems with inconsistent expression folding

If something is folded when binding a parameter it should be folded
when a the ECL is parsed, otherwise it can lead to inconsistencies.

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 9 سال پیش
والد
کامیت
82fa3fc105
7فایلهای تغییر یافته به همراه136 افزوده شده و 55 حذف شده
  1. 0 5
      ecl/hql/hqlerror.cpp
  2. 6 0
      ecl/hql/hqlerror.hpp
  3. 8 23
      ecl/hql/hqlexpr.cpp
  4. 87 2
      ecl/hql/hqlfold.cpp
  5. 3 0
      ecl/hql/hqlfold.hpp
  6. 30 23
      ecl/hql/hqlgram.y
  7. 2 2
      ecl/hql/hqlutil.cpp

+ 0 - 5
ecl/hql/hqlerror.cpp

@@ -240,11 +240,6 @@ IErrorReceiver * createThrowingErrorReceiver()
 
 //---------------------------------------------------------------------------------------------------------------------
 
-class HQL_API NullErrorReceiver : public ErrorReceiverSink
-{
-public:
-};
-
 IErrorReceiver * createNullErrorReceiver()
 {
     return new NullErrorReceiver;

+ 6 - 0
ecl/hql/hqlerror.hpp

@@ -60,6 +60,12 @@ private:
 
 //---------------------------------------------------------------------------------------------------------------------
 
+class HQL_API NullErrorReceiver : public ErrorReceiverSink
+{
+};
+
+//---------------------------------------------------------------------------------------------------------------------
+
 class IndirectErrorReceiver : public CInterfaceOf<IErrorReceiver>
 {
 public:

+ 8 - 23
ecl/hql/hqlexpr.cpp

@@ -11186,7 +11186,7 @@ protected:
             }
         case no_case:
             {
-                IHqlExpression * search = expr->queryChild(0);
+                OwnedHqlExpr search = transform(expr->queryChild(0));
                 if (!search->isConstant())
                     break;
                 OwnedHqlExpr folded = foldHqlExpression(search);
@@ -11200,31 +11200,16 @@ protected:
                     if (cur->getOperator() == no_mapto)
                     {
                         OwnedHqlExpr newcond = transform(cur->queryChild(0));
+                        if (!newcond->isConstant())
+                            break;
 
-                        if (newcond->isConstant())
-                            newcond.setown(foldHqlExpression(newcond));
-                        IValue * compareValue = newcond->queryValue();
-                        if (!compareValue)
+                        newcond.setown(foldHqlExpression(newcond));
+                        int result;
+                        if (!queryCompareConstantValues(result, folded, newcond))
                             break;
 
-                        if (searchType != newcond->queryType())
-                        {
-                            Owned<ITypeInfo> type = ::getPromotedECLCompareType(searchType, newcond->queryType());
-                            OwnedHqlExpr castSearch = ensureExprType(folded, type);
-                            OwnedHqlExpr castCompare = ensureExprType(newcond, type);
-                            IValue * castSearchValue = castSearch->queryValue();
-                            IValue * castCompareValue = castCompare->queryValue();
-                            if (!castSearchValue || !castCompareValue)
-                                break;
-
-                            if (castSearchValue->compare(castCompareValue) == 0)
-                                return transform(cur->queryChild(1));
-                        }
-                        else
-                        {
-                            if (searchValue->compare(compareValue) == 0)
-                                return transform(cur->queryChild(1));
-                        }
+                        if (result == 0)
+                            return transform(cur->queryChild(1));
                     }
                     else if (!cur->isAttribute())
                         return transform(cur);

+ 87 - 2
ecl/hql/hqlfold.cpp

@@ -6354,8 +6354,8 @@ IHqlExpression * CExprFolderTransformer::transformExpanded(IHqlExpression * expr
 
 IHqlExpression * foldHqlExpression(IHqlExpression * expr)
 {
-    Owned<IErrorReceiver> errorProcessor = createNullErrorReceiver();
-    return foldHqlExpression(*errorProcessor, expr);
+    NullErrorReceiver errorProcessor;
+    return foldHqlExpression(errorProcessor, expr);
 }
 
 IHqlExpression * foldHqlExpression(IErrorReceiver & errorProcessor, IHqlExpression * expr, ITemplateContext *templateContext, unsigned foldOptions)
@@ -6657,3 +6657,88 @@ extern HQLFOLD_API bool areExclusiveConditions(IHqlExpression * left, IHqlExpres
 
     return false;
 }
+
+
+bool queryCompareConstantValues(int & result, IHqlExpression * left, IHqlExpression * right)
+{
+    IValue * leftValue = left->queryValue();
+    IValue * rightValue = right->queryValue();
+    if (!leftValue || !rightValue)
+        return false;
+
+    ITypeInfo * leftType = left->queryType();
+    ITypeInfo * rightType = right->queryType();
+    if (leftType != rightType)
+    {
+        Owned<ITypeInfo> type = ::getPromotedECLCompareType(leftType, rightType);
+        OwnedHqlExpr castLeft = ensureExprType(left, type);
+        OwnedHqlExpr castRight = ensureExprType(right, type);
+        IValue * castLeftValue = castLeft->queryValue();
+        IValue * castRightValue = castRight->queryValue();
+        if (!castLeftValue || !castRightValue)
+            return false;
+
+        result = castLeftValue->compare(castRightValue);
+        return true;
+    }
+    else
+    {
+        result = leftValue->compare(rightValue);
+        return true;
+    }
+}
+
+IHqlExpression * foldConstantCaseExpr(IHqlExpression * expr)
+{
+    IHqlExpression * search = expr->queryChild(0);
+    if (!search->isConstant())
+        return LINK(expr);
+
+    OwnedHqlExpr foldedSearch = foldHqlExpression(search);
+    ForEachChildFrom(i, expr, 1)
+    {
+        IHqlExpression * cur = expr->queryChild(i);
+        if (cur->getOperator() == no_mapto)
+        {
+            IHqlExpression * mapValue = cur->queryChild(0);
+            if (!mapValue->isConstant())
+                return LINK(expr);
+
+            OwnedHqlExpr foldedValue = foldHqlExpression(mapValue);
+            int result;
+            if (!queryCompareConstantValues(result, foldedSearch, foldedValue))
+                return LINK(expr);
+
+            if (result == 0)
+                return LINK(cur->queryChild(1));
+        }
+        else if (!cur->isAttribute())
+            return LINK(cur);
+    }
+    return LINK(expr);
+}
+
+IHqlExpression * foldConstantMapExpr(IHqlExpression * expr)
+{
+    ForEachChild(i, expr)
+    {
+        IHqlExpression * cur = expr->queryChild(i);
+        if (cur->getOperator() == no_mapto)
+        {
+            IHqlExpression * mapValue = cur->queryChild(0);
+            if (!mapValue->isConstant())
+                return LINK(expr);
+
+            OwnedHqlExpr foldedValue = foldHqlExpression(mapValue);
+            IValue * value = foldedValue->queryValue();
+            if (!value)
+                return LINK(expr);
+
+            if (value->getBoolValue())
+                return LINK(cur->queryChild(1));
+        }
+        else if (!cur->isAttribute())
+            return LINK(cur);
+    }
+    return LINK(expr);
+}

+ 3 - 0
ecl/hql/hqlfold.hpp

@@ -57,6 +57,9 @@ extern HQLFOLD_API IHqlExpression * foldScopedHqlExpression(IErrorReceiver & err
 extern HQLFOLD_API void foldHqlExpression(IErrorReceiver & errorProcessor, HqlExprArray & tgt, HqlExprArray & src, unsigned options=0);
 extern HQLFOLD_API IHqlExpression * lowerCaseHqlExpr(IHqlExpression * expr);
 extern HQLFOLD_API IHqlExpression * foldExprIfConstant(IHqlExpression * expr);
+extern HQLFOLD_API bool queryCompareConstantValues(int & result, IHqlExpression * left, IHqlExpression * right);
+extern HQLFOLD_API IHqlExpression * foldConstantCaseExpr(IHqlExpression * expr);
+extern HQLFOLD_API IHqlExpression * foldConstantMapExpr(IHqlExpression * expr);
 
 extern HQLFOLD_API bool areExclusiveConditions(IHqlExpression * left, IHqlExpression * right);
 

+ 30 - 23
ecl/hql/hqlgram.y

@@ -2508,14 +2508,16 @@ actionStmt
                             HqlExprArray args;
                             $3.unwindCommaList(args);
                             args.append(*$5.getExpr());
-                            $$.setExpr(createValue(no_map, makeVoidType(), args), $1);
+                            OwnedHqlExpr expr = createValue(no_map, makeVoidType(), args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | MAP '(' mapActionSpec ')'
                         {
                             HqlExprArray args;
                             $3.unwindCommaList(args);
                             args.append(*createValue(no_null, makeVoidType()));
-                            $$.setExpr(createValue(no_map, makeVoidType(), args), $1);
+                            OwnedHqlExpr expr = createValue(no_map, makeVoidType(), args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseActionSpec ',' action ')'
                         {
@@ -2525,7 +2527,8 @@ actionStmt
                             parser->checkCaseForDuplicates(args, $6);
                             args.add(*$3.getExpr(),0);
                             args.append(*$8.getExpr());
-                            $$.setExpr(createValue(no_case, makeVoidType(), args), $1);
+                            OwnedHqlExpr expr = createValue(no_case, makeVoidType(), args);
+                            $$.setExpr(foldConstantCaseExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseActionSpec ')'
                         {
@@ -2535,7 +2538,8 @@ actionStmt
                             parser->checkCaseForDuplicates(args, $6);
                             args.add(*$3.getExpr(),0);
                             args.append(*createValue(no_null, makeVoidType()));
-                            $$.setExpr(createValue(no_case, makeVoidType(), args), $1);
+                            OwnedHqlExpr expr = createValue(no_case, makeVoidType(), args);
+                            $$.setExpr(foldConstantCaseExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList action ')'
                         {
@@ -5773,7 +5777,8 @@ primexpr1
                             $3.unwindCommaList(args);
                             ITypeInfo * retType = parser->promoteMapToSameType(args, $5);
                             args.append(*$5.getExpr());
-                            $$.setExpr(createValue(no_map, retType, args));
+                            OwnedHqlExpr expr = createValue(no_map, retType, args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseSpec ',' expression ')'
                         {
@@ -5785,7 +5790,8 @@ primexpr1
                             ITypeInfo * retType = parser->promoteCaseToSameType($3, args, $8);
                             args.add(*$3.getExpr(),0);
                             args.append(*$8.getExpr());
-                            $$.setExpr(createValue(no_case, retType, args));
+                            OwnedHqlExpr expr = createValue(no_case, retType, args);
+                            $$.setExpr(foldConstantCaseExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList expression ')'
                         {
@@ -7611,8 +7617,8 @@ simpleDictionary
                                 parser->checkRecordTypesMatch(cur, elseDict, $5);
                             }
                             args.append(*elseDict);
-                            $$.setExpr(::createDictionary(no_map, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createDictionary(no_map, args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | MAP '(' mapDictionarySpec ')'
                         {
@@ -7629,8 +7635,8 @@ simpleDictionary
                                 parser->checkRecordTypesMatch(cur, elseDict, $1);
                             }
                             args.append(*elseDict);
-                            $$.setExpr(::createDictionary(no_map, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createDictionary(no_map, args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseDictionarySpec ',' dictionary ')'
                         {
@@ -7646,8 +7652,8 @@ simpleDictionary
                             }
                             args.add(*$3.getExpr(),0);
                             args.append(*elseDict);
-                            $$.setExpr(::createDataset(no_case, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createDictionary(no_case, args);
+                            $$.setExpr(foldConstantCaseExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseDictionarySpec ')'
                         {
@@ -7667,8 +7673,8 @@ simpleDictionary
                             }
                             args.add(*$3.getExpr(),0);
                             args.append(*elseDict);
-                            $$.setExpr(::createDictionary(no_case, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createDictionary(no_case, args);
+                            $$.setExpr(foldConstantCaseExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList dictionary ')'
                         {
@@ -9091,8 +9097,8 @@ simpleDataSet
                             $3.unwindCommaList(args);
                             parser->ensureMapToRecordsMatch(elseExpr, args, $5, false);
                             args.append(*elseExpr.getClear());
-                            $$.setExpr(::createDataset(no_map, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createDataset(no_map, args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | MAP '(' mapDatasetSpec ')'
                         {
@@ -9106,8 +9112,8 @@ simpleDataSet
 
                             parser->ensureMapToRecordsMatch(elseExpr, args, $3, false);
                             args.append(*elseExpr.getClear());
-                            $$.setExpr(::createDataset(no_map, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createDataset(no_map, args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseDatasetSpec ',' dataSet ')'
                         {
@@ -9121,8 +9127,8 @@ simpleDataSet
 
                             args.add(*$3.getExpr(),0);
                             args.append(*elseExpr.getClear());
-                            $$.setExpr(::createDataset(no_case, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createDataset(no_case, args);
+                            $$.setExpr(foldConstantCaseExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseDatasetSpec ')'
                         {
@@ -9433,8 +9439,8 @@ simpleDataSet
                             parser->ensureMapToRecordsMatch(elseExpr, args, $5, true);
 
                             args.append(*elseExpr.getClear());
-                            $$.setExpr(::createRow(no_map, args));
-                            $$.setPosition($1);
+                            OwnedHqlExpr expr = createRow(no_map, args);
+                            $$.setExpr(foldConstantMapExpr(expr), $1);
                         }
     | CASE '(' expression ',' beginList caseDatarowSpec ',' dataRow ')'
                         {
@@ -9448,7 +9454,8 @@ simpleDataSet
 
                             args.add(*$3.getExpr(),0);
                             args.append(*elseExpr.getClear());
-                            $$.setExpr(::createRow(no_case, args), $1);
+                            OwnedHqlExpr expr = createRow(no_case, args);
+                            $$.setExpr(foldConstantCaseExpr(expr), $1);
                         }
     | WHEN '(' dataSet ',' action sideEffectOptions ')'
                         {

+ 2 - 2
ecl/hql/hqlutil.cpp

@@ -1293,8 +1293,8 @@ IHqlExpression * createImpureOwn(IHqlExpression * expr)
 
 IHqlExpression * getNormalizedFilename(IHqlExpression * filename)
 {
-    Owned<IErrorReceiver> errorProcessor = createNullErrorReceiver();
-    OwnedHqlExpr folded = foldHqlExpression(*errorProcessor, filename, NULL, HFOloseannotations);
+    NullErrorReceiver errorProcessor;
+    OwnedHqlExpr folded = foldHqlExpression(errorProcessor, filename, NULL, HFOloseannotations);
     return lowerCaseHqlExpr(folded);
 }