HPCC-10164 Don't substitute constants into ATMOST join conditions

This should be improved later (see HPCC-10166), but this fixes the compile
errors.

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ecl/hql/hqlfold.cpp

@@ -4016,9 +4016,7 @@ public:
         case no_attr_expr:
             {
                 IAtom * name = expr->queryName();
-                if (name == atmostAtom)
-                    return LINK(expr);
-                else if (name == _selectors_Atom)
+                if (name == _selectors_Atom)
                 {
                     HqlExprArray args;
                     ForEachChild(i, expr)
@@ -5120,12 +5118,18 @@ IHqlExpression * CExprFolderTransformer::percolateConstants(IHqlExpression * exp
                         updated.setown(removeProperty(updated, atmostAtom));
                     else
                     {
-                        HqlExprArray args;
-                        unwindChildren(args, updated);
-                        args.replace(*LINK(oldCond), 2);
-                        removeProperty(args, atmostAtom);
-                        args.append(*LINK(atmost));
-                        updated.setown(updated->clone(args));
+                        //KEYED joins support ATMOST and RIGHT.xxx = value
+                        if (!isKeyedJoin(updated) && joinHasRightOnlyHardMatch(updated, false))
+                        {
+                            HqlExprArray args;
+                            unwindChildren(args, updated);
+                            args.replace(*LINK(oldCond), 2);
+                            removeProperty(args, atmostAtom);
+                            args.append(*LINK(atmost));
+                            updated.setown(updated->clone(args));
+                        }
+                        else
+                            updated.setown(appendOwnedOperand(updated, createAttribute(_conditionFolded_Atom)));
                     }
                 }
                 //otherwise this might convert to an all join, accept variants that are supported by all joins

ecl/hql/hqlgram2.cpp

@@ -7735,7 +7735,7 @@ void HqlGram::checkJoinFlags(const attribute &err, IHqlExpression * join)
         if (keep && !isMany)
             reportError(ERR_BADKIND_LOOKUPJOIN, err, "%s joins do not support KEEP", joinText);
         if (join->hasAttribute(atmostAtom) && !isMany)
-            reportError(ERR_BADKIND_LOOKUPJOIN, err, "%s joins do not support ATMOST", joinText);
+            reportError(ERR_BADKIND_LOOKUPJOIN, err, "Non-Many %s joins do not support ATMOST", joinText);
         if (rowLimit && !isMany)
             reportError(ERR_BADKIND_LOOKUPJOIN, err, "%s joins do not support LIMIT (they can only match 1 entry)", joinText);
         if (isKey(join->queryChild(1)))

ecl/hql/hqlopt.cpp

@@ -3688,6 +3688,7 @@ IHqlExpression * CTreeOptimizer::doCreateTransformed(IHqlExpression * transforme
                     {
                         if (!isPureActivity(child) || child->queryAttribute(_countProject_Atom) || child->hasAttribute(prefetchAtom))
                             break;
+
                         IHqlExpression * transform = queryNewColumnProvider(child);
                         if (transformContainsSkip(transform) || !isSimpleTransformToMergeWith(transform))
                             break;
@@ -3724,7 +3725,18 @@ IHqlExpression * CTreeOptimizer::doCreateTransformed(IHqlExpression * transforme
                                 args.append(*createAttribute(allAtom));
                             DBGLOG("Optimizer: Merge %s and %s", queryNode0Text(transformed), queryNode1Text(child));
                             noteUnused(child);
-                            return transformed->clone(args);
+                            OwnedHqlExpr merged = transformed->clone(args);
+
+                            //Substituting constants into LEFT join expression can cause problems for the ATMOST join
+                            //Only keyed joins currently support it.
+                            if (transformed->hasAttribute(atmostAtom) && !isKeyedJoin(transformed))
+                            {
+                                if (joinHasRightOnlyHardMatch(merged, false))
+                                    merged.clear();
+                            }
+
+                            if (merged)
+                                return merged.getClear();
                         }
                         break;
                     }

ecl/hql/hqlutil.cpp

@@ -939,6 +939,16 @@ IHqlExpression * JoinOrderSpotter::doFindJoinSortOrders(IHqlExpression * conditi
             return NULL;
         return LINK(condition);
 
+    case no_assertkeyed:
+    {
+        OwnedHqlExpr ret = doFindJoinSortOrders(l, allowSlidingMatch, matched);
+        if (ret)
+            return createValue(no_assertkeyed, condition->getType(), ret.getClear());
+        return NULL;
+    }
+    case no_assertwild:
+        return NULL;
+
     default:
         return LINK(condition);
     }
@@ -1043,6 +1053,7 @@ void JoinOrderSpotter::findImplicitBetween(IHqlExpression * condition, HqlExprAr
 JoinSortInfo::JoinSortInfo()
 {
     conditionAllEqualities = false;
+    hasRightNonEquality = false;
 }
 
 void JoinSortInfo::findJoinSortOrders(IHqlExpression * condition, IHqlExpression * leftDs, IHqlExpression * rightDs, IHqlExpression * seq, bool allowSlidingMatch)
@@ -1059,6 +1070,12 @@ void JoinSortInfo::findJoinSortOrders(IHqlExpression * condition, IHqlExpression
     
     extraMatch.setown(spotter.doFindJoinSortOrders(condition, allowSlidingMatch, matched));
     conditionAllEqualities = (extraMatch == NULL);
+    if (extraMatch)
+    {
+        OwnedHqlExpr right = createSelector(no_right, rightDs, seq);
+        if (extraMatch->usesSelector(right))
+            hasRightNonEquality = true;
+    }
 
     //Support for legacy syntax where x[n..*] was present in join and atmost condition
     //Ensure they are tagged as optional join fields.
@@ -1118,7 +1135,8 @@ void JoinSortInfo::findJoinSortOrders(IHqlExpression * expr, bool allowSlidingMa
 
     OwnedHqlExpr fuzzy, hard;
     splitFuzzyCondition(expr->queryChild(2), atmost.required, fuzzy, hard);
-    findJoinSortOrders(hard, lhs, rhs, selSeq, allowSlidingMatch);
+    if (hard)
+        findJoinSortOrders(hard, lhs, rhs, selSeq, allowSlidingMatch);
 
     extraMatch.setown(extendConditionOwn(no_and, extraMatch.getClear(), fuzzy.getClear()));
 }
@@ -1145,6 +1163,13 @@ void JoinSortInfo::initSorts()
 }
 
 
+extern HQL_API bool joinHasRightOnlyHardMatch(IHqlExpression * expr, bool allowSlidingMatch)
+{
+    JoinSortInfo joinInfo;
+    joinInfo.findJoinSortOrders(expr, false);
+    return joinInfo.hasHardRightNonEquality();
+}
+
 IHqlExpression * createImpureOwn(IHqlExpression * expr)
 {
     return createValue(no_impure, expr->getType(), expr);

ecl/hql/hqlutil.hpp

@@ -701,6 +701,7 @@ public:
 
     inline bool hasRequiredEqualities() const { return leftReq.ordinality() != 0; }
     inline bool hasOptionalEqualities() const { return leftOpt.ordinality() != 0; }
+    inline bool hasHardRightNonEquality() const { return hasRightNonEquality; }
     inline const HqlExprArray & queryLeftReq() { return leftReq; }
     inline const HqlExprArray & queryRightReq() { return rightReq; }
     inline const HqlExprArray & queryLeftOpt() { return leftOpt; }
@@ -716,6 +717,7 @@ public:
     OwnedHqlExpr extraMatch;
     HqlExprArray slidingMatches;
     bool conditionAllEqualities;
+    bool hasRightNonEquality;
 protected:
     HqlExprArray leftReq;
     HqlExprArray leftOpt;
@@ -725,4 +727,6 @@ protected:
     HqlExprArray rightSorts;
 };
 
+extern HQL_API bool joinHasRightOnlyHardMatch(IHqlExpression * expr, bool allowSlidingMatch);
+
 #endif

ecl/hqlcpp/hqlhtcpp.cpp

@@ -11539,7 +11539,7 @@ ABoundActivity * HqlCppTranslator::doBuildActivityJoinOrDenormalize(BuildCtx & c
     JoinSortInfo joinInfo;
     joinInfo.findJoinSortOrders(expr, slidingAllowed);
 
-    if (atmostAttr && !joinInfo.conditionAllEqualities)
+    if (atmostAttr && joinInfo.hasHardRightNonEquality())
     {
         if (isAllJoin)
             allowAllToLookupConvert = false;

ecl/regress/issue10164.ecl

@@ -0,0 +1,23 @@
+namesRecord :=
+            RECORD
+string20        surname;
+string10        forename;
+integer2        age := 25;
+            END;
+
+xRecord :=
+            RECORD
+string20        surname;
+string10        forename;
+            END;
+
+namesTable := index(namesRecord,'i');
+
+
+x := DATASET(['Smith','Blogs'], { string20 surname, });
+
+p := PROJECT(NOFOLD(x), transform(xRecord, SELF.forename := 'John'; SELF := LEFT));
+
+j := JOIN(p, namesTable, LEFT.surname = RIGHT.surname AND LEFT.forename = RIGHT.forename, ATMOST(100), LOOKUP, MANY);
+
+output(j,,'out.d00',overwrite);