Merge pull request #7260 from ghalliday/issue13429

HPCC-13429 Better error for scalar output inside child query

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

common/thorhelper/roxiehelper.cpp

@@ -1209,8 +1209,10 @@ void FlushingStringBuffer::startDataset(const char *elementName, const char *res
 
 void FlushingStringBuffer::startScalar(const char *resultName, unsigned sequence)
 {
+    if (!s.length())
+        throw MakeStringException(0, "Attempt to output scalar ('%s',%d) multiple times", resultName ? resultName : "", (int)sequence);
+
     CriticalBlock b(crit);
-    assertex(!s.length());
     name.clear().append(resultName ? resultName : "Dataset");
 
     sequenceNumber = 0;
@@ -1311,8 +1313,10 @@ void FlushingJsonBuffer::startDataset(const char *elementName, const char *resul
 
 void FlushingJsonBuffer::startScalar(const char *resultName, unsigned sequence)
 {
+    if (!s.length())
+        throw MakeStringException(0, "Attempt to output scalar ('%s',%d) multiple times", resultName ? resultName : "", (int)sequence);
+
     CriticalBlock b(crit);
-    assertex(!s.length());
     name.set(resultName ? resultName : "Dataset");
 
     sequenceNumber = 0;

ecl/hqlcpp/hqlcerrors.hpp

@@ -263,6 +263,7 @@
 #define HQLWRN_NoThorContextDependent           4544
 #define HQLWRN_OnlyLocalMergeJoin               4545
 #define HQLWRN_WorkflowDependParameter          4546
+#define HQLWRN_OutputScalarInsideChildQuery     4547
 
 //Temporary errors
 #define HQLERR_OrderOnVarlengthStrings          4601
@@ -542,6 +543,7 @@
 #define HQLWRN_NoThorContextDependent_Text      "NOTHOR expression%s appears to access a parent dataset - this may cause a dataset not active error"
 #define HQLWRN_OnlyLocalMergeJoin_Text          "Only LOCAL versions of %s are currently supported on THOR"
 #define HQLWRN_WorkflowDependParameter_Text     "Workflow action %s appears to be dependent upon a parameter"
+#define HQLWRN_OutputScalarInsideChildQuery_Text "Output(%s) of single value inside a child query has undefined behaviour"
 
 #define HQLERR_DistributionVariableLengthX_Text "DISTRIBUTION does not support variable length field '%s'"
 #define HQLERR_DistributionUnsupportedTypeXX_Text "DISTRIBUTION does not support field '%s' with type %s"

ecl/hqlcpp/hqlhtcpp.cpp

@@ -7635,6 +7635,13 @@ void HqlCppTranslator::doBuildStmtSetResult(BuildCtx & ctx, IHqlExpression * exp
         graphLabel.set(text.str());
     }
 
+    if (insideChildQuery(ctx))
+    {
+        StringBuffer description;
+        getStoredDescription(description, seq, name, true);
+        reportWarning(CategoryUnusual, SeverityError, queryLocation(expr), ECODETEXT(HQLWRN_OutputScalarInsideChildQuery), description.str());
+    }
+
     if (cluster)
         pushCluster(subctx, cluster->queryChild(0));
 
@@ -17852,6 +17859,13 @@ ABoundActivity * HqlCppTranslator::doBuildActivitySetResult(BuildCtx & ctx, IHql
 
     buildInstancePrefix(instance);
 
+    if (insideChildQuery(ctx))
+    {
+        StringBuffer description;
+        getStoredDescription(description, sequence, name, true);
+        reportWarning(CategoryUnusual, SeverityError, queryLocation(expr), ECODETEXT(HQLWRN_OutputScalarInsideChildQuery), description.str());
+    }
+
     noteResultDefined(ctx, instance, sequence, name, isRoot);
     if (attribute->isDatarow())
         attribute.setown(::ensureSerialized(attribute, diskAtom));

ecl/regress/scalarchild.ecl

@@ -0,0 +1,18 @@
+//fail
+
+idRec := {unsigned id };
+
+ds := DATASET(10, TRANSFORM(idRec, SELF.id := HASH64(COUNTER)));
+
+f(unsigned id) := FUNCTION
+    o := OUTPUT(id);
+    RETURN WHEN(id * 2, o);
+END;
+
+idRec t(idRec l) := TRANSFORM
+    SELF.id := f(l.id);
+END;
+
+p := PROJECT(NOFOLD(ds), t(LEFT));
+
+output(p);

roxie/ccd/ccdlistener.cpp

@@ -1767,7 +1767,7 @@ readAnother:
             }
             else
             {
-                fprintf(stderr, "EXCEPTION: %s", error.str());
+                fprintf(stderr, "EXCEPTION: %s\n", error.str());
             }
             E->Release();
         }

thorlcr/graph/thgraphslave.cpp

@@ -920,22 +920,27 @@ class CThorCodeContextSlave : public CThorCodeContextBase, implements IEngineCon
     mptag_t mptag;
     Owned<IDistributedFileTransaction> superfiletransaction;
 
+    void invalidSetResult(const char * name, unsigned seq)
+    {
+        throw MakeStringException(0, "Attempt to output result ('%s',%d) from a child query", name ? name : "", (int)seq);
+    }
+
 public:
     CThorCodeContextSlave(CJobBase &job, ILoadedDllEntry &querySo, IUserDescriptor &userDesc, mptag_t _mptag) : CThorCodeContextBase(job, querySo, userDesc), mptag(_mptag)
     {
     }
-    virtual void setResultBool(const char *name, unsigned sequence, bool value) { throwUnexpected(); }
-    virtual void setResultData(const char *name, unsigned sequence, int len, const void * data) { throwUnexpected(); }
-    virtual void setResultDecimal(const char * stepname, unsigned sequence, int len, int precision, bool isSigned, const void *val) { throwUnexpected(); } 
-    virtual void setResultInt(const char *name, unsigned sequence, __int64 value, unsigned size) { throwUnexpected(); }
-    virtual void setResultRaw(const char *name, unsigned sequence, int len, const void * data) { throwUnexpected(); }
-    virtual void setResultReal(const char * stepname, unsigned sequence, double value) { throwUnexpected(); }
-    virtual void setResultSet(const char *name, unsigned sequence, bool isAll, size32_t len, const void * data, ISetToXmlTransformer * transformer) { throwUnexpected(); }
-    virtual void setResultString(const char *name, unsigned sequence, int len, const char * str) { throwUnexpected(); }
-    virtual void setResultUInt(const char *name, unsigned sequence, unsigned __int64 value, unsigned size) { throwUnexpected(); }
-    virtual void setResultUnicode(const char *name, unsigned sequence, int len, UChar const * str) { throwUnexpected(); }
-    virtual void setResultVarString(const char * name, unsigned sequence, const char * value) { throwUnexpected(); }
-    virtual void setResultVarUnicode(const char * name, unsigned sequence, UChar const * value) { throwUnexpected(); }
+    virtual void setResultBool(const char *name, unsigned sequence, bool value) { invalidSetResult(name, sequence); }
+    virtual void setResultData(const char *name, unsigned sequence, int len, const void * data) { invalidSetResult(name, sequence); }
+    virtual void setResultDecimal(const char * stepname, unsigned sequence, int len, int precision, bool isSigned, const void *val) { invalidSetResult(stepname, sequence); }
+    virtual void setResultInt(const char *name, unsigned sequence, __int64 value, unsigned size) { invalidSetResult(name, sequence); }
+    virtual void setResultRaw(const char *name, unsigned sequence, int len, const void * data) { invalidSetResult(name, sequence); }
+    virtual void setResultReal(const char * stepname, unsigned sequence, double value) { invalidSetResult(stepname, sequence); }
+    virtual void setResultSet(const char *name, unsigned sequence, bool isAll, size32_t len, const void * data, ISetToXmlTransformer * transformer) { invalidSetResult(name, sequence); }
+    virtual void setResultString(const char *name, unsigned sequence, int len, const char * str) { invalidSetResult(name, sequence); }
+    virtual void setResultUInt(const char *name, unsigned sequence, unsigned __int64 value, unsigned size) { invalidSetResult(name, sequence); }
+    virtual void setResultUnicode(const char *name, unsigned sequence, int len, UChar const * str) { invalidSetResult(name, sequence); }
+    virtual void setResultVarString(const char * name, unsigned sequence, const char * value) { invalidSetResult(name, sequence); }
+    virtual void setResultVarUnicode(const char * name, unsigned sequence, UChar const * value) { invalidSetResult(name, sequence); }
 
     virtual bool getResultBool(const char * name, unsigned sequence) { throwUnexpected(); }
     virtual void getResultData(unsigned & tlen, void * & tgt, const char * name, unsigned sequence) { throwUnexpected(); }