Merge pull request #328 from ghalliday/bug86144

BUG: #86144 Report an error if SKIP is used in ROW

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

ecl/hqlcpp/hqlcerrors.hpp

@@ -204,6 +204,7 @@
 #define HQLERR_CouldNotDetermineMinSize         4181
 #define HQLERR_OutsideGroupAggregate            4182
 #define HQLERR_ResourceAddAfterFinalManifest    4183
+#define HQLERR_SkipInsideCreateRow              4184
 
 //Warnings....
 #define HQLWRN_PersistDataNotLikely             4500
@@ -479,6 +480,7 @@
 #define HQLERR_CouldNotDetermineMinSize_Text    "Cannot determine the minimum size of the expression"
 #define HQLERR_OutsideGroupAggregate_Text       "%s used outside of a TABLE aggregation"
 #define HQLERR_ResourceAddAfterFinalManifest_Text "%s resource added after manifest was finalized"
+#define HQLERR_SkipInsideCreateRow_Text         "SKIP inside a ROW(<transform>) not supported.  It is only allowed in a DATASET transform."
 
 //Warnings.
 #define HQLWRN_CannotRecreateDistribution_Text  "Cannot recreate the distribution for a persistent dataset"

ecl/hqlcpp/hqlcpp.ipp

@@ -1549,7 +1549,7 @@ public:
     ABoundActivity * doBuildActivityCombineGroup(BuildCtx & ctx, IHqlExpression * expr);
     ABoundActivity * doBuildActivityCompoundSelectNew(BuildCtx & ctx, IHqlExpression * expr);
     ABoundActivity * doBuildActivityConcat(BuildCtx & ctx, IHqlExpression * expr);
-    ABoundActivity * doBuildActivityCreateRow(BuildCtx & ctx, IHqlExpression * expr);
+    ABoundActivity * doBuildActivityCreateRow(BuildCtx & ctx, IHqlExpression * expr, bool isDataset);
     ABoundActivity * doBuildActivityXmlRead(BuildCtx & ctx, IHqlExpression * expr);
     ABoundActivity * doBuildActivityDedup(BuildCtx & ctx, IHqlExpression * expr);
     ABoundActivity * doBuildActivityDefineSideEffect(BuildCtx & ctx, IHqlExpression * expr);

ecl/hqlcpp/hqlhtcpp.cpp

@@ -5967,7 +5967,7 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
                 {
                     OwnedHqlExpr row = expr->cloneAllAnnotations(expr->queryChild(0));  // preserve any position information....
                     if ((getNumActivityArguments(expr) == 0) && canProcessInline(&ctx, row))
-                        result = doBuildActivityCreateRow(ctx, row);
+                        result = doBuildActivityCreateRow(ctx, row, false);
                     else
                         result = buildCachedActivity(ctx, row);
                     break;
@@ -5992,7 +5992,7 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
                 break;
             case no_null:
                 if (expr->isDatarow())
-                    result = doBuildActivityCreateRow(ctx, expr);
+                    result = doBuildActivityCreateRow(ctx, expr, false);
                 else
                     result = doBuildActivityNull(ctx, expr, isRoot);
                 break;
@@ -6167,7 +6167,7 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
                 result = doBuildActivityProject(ctx, expr);
                 break;
             case no_createrow:
-                result = doBuildActivityCreateRow(ctx, expr);
+                result = doBuildActivityCreateRow(ctx, expr, false);
                 break;
             case no_newusertable:
                 result = doBuildActivityProject(ctx, expr);
@@ -16099,7 +16099,7 @@ ABoundActivity * HqlCppTranslator::doBuildActivityTempTable(BuildCtx & ctx, IHql
 
 
 //NB: Also used to create row no_null as an activity.
-ABoundActivity * HqlCppTranslator::doBuildActivityCreateRow(BuildCtx & ctx, IHqlExpression * expr)
+ABoundActivity * HqlCppTranslator::doBuildActivityCreateRow(BuildCtx & ctx, IHqlExpression * expr, bool isDataset)
 {
     bool valuesAreConstant = false;
     StringBuffer valueText;
@@ -16119,6 +16119,8 @@ ABoundActivity * HqlCppTranslator::doBuildActivityCreateRow(BuildCtx & ctx, IHql
             }
             valuesAreConstant = true;
         }
+        if (!isDataset && containsSkip(transform))
+            reportError(queryLocation(transform), ECODETEXT(HQLERR_SkipInsideCreateRow));
     }
     else if (op == no_null)
     {
@@ -16150,7 +16152,9 @@ ABoundActivity * HqlCppTranslator::doBuildActivityCreateRow(BuildCtx & ctx, IHql
     BoundRow * selfCursor = bindSelf(funcctx, instance->dataset, "crSelf");
     IHqlExpression * self = selfCursor->querySelector();
 
-    associateSkipReturnMarker(funcctx, queryBoolExpr(false), selfCursor);
+    if (isDataset)
+        associateSkipReturnMarker(funcctx, queryBoolExpr(false), selfCursor);
+
     buildAssign(funcctx, self, expr);
     buildReturnRecordSize(funcctx, selfCursor);
 
@@ -16169,7 +16173,7 @@ ABoundActivity * HqlCppTranslator::doBuildActivityInlineTable(BuildCtx & ctx, IH
     {
         OwnedHqlExpr rowValue = createRow(no_createrow, LINK(values->queryChild(0)));
         OwnedHqlExpr row = expr->cloneAllAnnotations(rowValue);
-        return doBuildActivityCreateRow(ctx, row);
+        return doBuildActivityCreateRow(ctx, row, true);
     }
 
     Owned<ActivityInstance> instance = new ActivityInstance(*this, ctx, TAKtemptable, expr,"TempTable");