Browse Source

Merge branch 'candidate-7.2.0' into candidate-7.2.x

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>
Richard Chapman 6 years ago
parent
commit
c6384cb8a0
5 changed files with 68 additions and 41 deletions
  1. 1 1
      ecl/hql/hqlfold.cpp
  2. 47 28
      ecl/hqlcpp/hqlsource.cpp
  3. 1 0
      ecl/hthor/hthor.cpp
  4. 1 1
      plugins/javaembed/javaembed.cpp
  5. 18 11
      roxie/ccd/ccdserver.cpp

+ 1 - 1
ecl/hql/hqlfold.cpp

@@ -1479,7 +1479,7 @@ class DummyContext: implements ICodeContext
     virtual char *getGroupName() { throwUnexpected(); } // caller frees return string.
     virtual char *getJobName() { throwUnexpected(); } // caller frees return string.
     virtual char *getJobOwner() { throwUnexpected(); } // caller frees return string.
-    virtual unsigned getNodeNum() { throwUnexpected(); }
+    virtual unsigned getNodeNum() { return 0; }
     virtual unsigned getNodes() { throwUnexpected(); }
     virtual char *getOS() { throwUnexpected(); } // caller frees return string
     virtual char *getPlatform() { throwUnexpected(); } // caller frees return string.

+ 47 - 28
ecl/hqlcpp/hqlsource.cpp

@@ -2976,46 +2976,65 @@ void DiskReadBuilderBase::extractMonitors(IHqlExpression * ds, SharedHqlExpr & u
     //KEYED filters can only currently be implemented for binary files - not csv, xml or pipe....
     if (queryTableMode(tableExpr) == no_flat)
     {
-        OwnedHqlExpr implicitFilter;
-        ForEachItemIn(i, conds)
+        if (translator.queryOptions().implicitKeyedDiskFilter)
         {
-            IHqlExpression * filter = &conds.item(i);
-            if (isSourceInvariant(ds, filter))                  // more actually isSourceInvariant.
-                extendConditionOwn(globalGuard, no_and, LINK(filter));
-            else
+            HqlExprArray newconds;
+            ForEachItemIn(i, conds)
             {
-                node_operator op = filter->getOperator();
-                switch (op)
+                IHqlExpression * filter = &conds.item(i);
+                if (isSourceInvariant(ds, filter))                  // more actually isSourceInvariant.
+                    extendConditionOwn(globalGuard, no_and, LINK(filter));
+                else
+                    newconds.append(OLINK(*filter));
+            }
+
+            OwnedHqlExpr extraFilter;
+            monitors.extractFilters(newconds, extraFilter);
+            appendFilter(unkeyedFilter, extraFilter);
+        }
+        else
+        {
+            OwnedHqlExpr implicitFilter;
+            ForEachItemIn(i, conds)
+            {
+                IHqlExpression * filter = &conds.item(i);
+                if (isSourceInvariant(ds, filter))                  // more actually isSourceInvariant.
+                    extendConditionOwn(globalGuard, no_and, LINK(filter));
+                else
                 {
-                case no_assertkeyed:
-                case no_assertwild:
+                    node_operator op = filter->getOperator();
+                    switch (op)
                     {
-                        //MORE: This needs to test that the fields are at fixed offsets, fixed length, and collatable.
-                        OwnedHqlExpr extraFilter;
-                        monitors.extractFilters(filter, extraFilter);
+                    case no_assertkeyed:
+                    case no_assertwild:
+                        {
+                            //MORE: This needs to test that the fields are at fixed offsets, fixed length, and collatable.
+                            OwnedHqlExpr extraFilter;
+                            monitors.extractFilters(filter, extraFilter);
 
-                        //NB: Even if it is keyed then (part of) the test condition might be duplicated.
-                        appendFilter(unkeyedFilter, extraFilter);
+                            //NB: Even if it is keyed then (part of) the test condition might be duplicated.
+                            appendFilter(unkeyedFilter, extraFilter);
+                            break;
+                        }
+                    default:
+                        // Add this condition to the catchall filter
+                        appendFilter(implicitFilter, filter);
                         break;
                     }
-                default:
-                    // Add this condition to the catchall filter
-                    appendFilter(implicitFilter, filter);
-                    break;
                 }
             }
-        }
 
-        if (implicitFilter)
-        {
-            if (translator.queryOptions().implicitKeyedDiskFilter && !monitors.isKeyed())
+            if (implicitFilter)
             {
-                OwnedHqlExpr extraFilter;
-                monitors.extractFilters(implicitFilter.get(), extraFilter);
-                appendFilter(unkeyedFilter, extraFilter);
+                if (translator.queryOptions().implicitKeyedDiskFilter && !monitors.isKeyed())
+                {
+                    OwnedHqlExpr extraFilter;
+                    monitors.extractFilters(implicitFilter.get(), extraFilter);
+                    appendFilter(unkeyedFilter, extraFilter);
+                }
+                else
+                    appendFilter(unkeyedFilter, implicitFilter);
             }
-            else
-                appendFilter(unkeyedFilter, implicitFilter);
         }
     }
     else

+ 1 - 0
ecl/hthor/hthor.cpp

@@ -8485,6 +8485,7 @@ bool CHThorDiskReadBaseActivity::openNext()
         partNum++;
         if (checkOpenedFile(file.str(), NULL))
         {
+            actualFilter.appendFilters(fieldFilters);
             opened = true;
             return true;
         }

+ 1 - 1
plugins/javaembed/javaembed.cpp

@@ -3071,7 +3071,7 @@ public:
                 val++;
                 if (stricmp(optName, "classpath")==0)
                     lclassPath.append(';').append(val);
-                if (strieq(optName, "globalscope"))
+                else if (strieq(optName, "globalscope"))
                     globalScopeKey.set(val);
                 else if (strieq(optName, "persist"))
                 {

+ 18 - 11
roxie/ccd/ccdserver.cpp

@@ -8860,7 +8860,8 @@ public:
     CriticalSection crit2;
     unsigned tailIdx;
     unsigned headIdx;
-    Owned<IException> error;
+    Owned<IException> readError;
+    Owned<IException> startError;
 
     class OutputAdaptor : implements IEngineRowStream, implements IFinalRoxieInput, public CInterface
     {
@@ -9079,8 +9080,8 @@ public:
             if (curIdx == headIdx) // test again now that we have it
             {
                 ActivityTimer t(totalCycles, timeActivities); // NOTE - time spent waiting for crit not included here. But it will have been included on the totalTime of the person holding the crit, so that is right
-                if (error)
-                    throw error.getLink();
+                if (readError)
+                    throw readError.getLink();  // Read exceptions are only throw if you try to read past the row where they happened
 
                 try
                 {
@@ -9109,13 +9110,13 @@ public:
 #ifdef TRACE_SPLIT
                     CTXLOG("spill %d caught exception", activityId);
 #endif
-                    error.set(E);
+                    readError.set(E);
                     throw;
                 }
                 catch (...)
                 {
                     IException *E = MakeStringException(ROXIE_INTERNAL_ERROR, "Unknown exception caught in CRoxieServerThroughSpillActivity::readBuffered");
-                    error.set(E);
+                    readError.set(E);
                     throw E;
                 }
             }
@@ -9150,8 +9151,8 @@ public:
     virtual void start(unsigned oid, unsigned parentExtractSize, const byte *parentExtract, bool paused)
     {
         CriticalBlock b(crit);
-        if (error)
-            throw error.getLink();
+        if (startError)
+            throw startError.getLink();
         if (collectFactoryStatistics)
             factory->noteStarted(oid);
         if (traceStartStop)
@@ -9162,7 +9163,8 @@ public:
                 initOutputs();
             tailIdx = 0;
             headIdx = 0;
-            error.clear();
+            startError.clear();
+            readError.clear();
             try
             {
                 CRoxieServerActivity::start(parentExtractSize, parentExtract, paused);
@@ -9172,13 +9174,13 @@ public:
 #ifdef TRACE_SPLIT
                 CTXLOG("spill %d caught exception in start", activityId);
 #endif
-                error.set(E);
+                startError.set(E);
                 throw;
             }
             catch (...)
             {
                 IException *E = MakeStringException(ROXIE_INTERNAL_ERROR, "Unknown exception caught in CRoxieServerThroughSpillActivity::start");
-                error.set(E);
+                startError.set(E);
                 throw E;
             }
         }
@@ -9239,7 +9241,8 @@ public:
         activeOutputs = numOutputs;
         while (buffer.ordinality())
             ReleaseRoxieRow(buffer.dequeue());
-        error.clear();
+        startError.clear();
+        readError.clear();
         if (state != STATEreset) // make sure input is only reset once
             CRoxieServerActivity::reset();
     };
@@ -20067,6 +20070,10 @@ public:
     {
     }
 
+    // MORE - you could argue we should catch exceptions in START too, though not clear what to do with them
+    // See CRoxieServerSkipCatchActivity::start
+    // This is less significant now that we properly distinguish start exceptions from read exceptions in ThroughSpill activity
+
     virtual const void *nextRow()
     {
         ActivityTimer t(totalCycles, timeActivities);