소스 검색

HPCC-19745 Move warning related to grouping in tables out of parser

Signed-off-by: Shamser Ahmed <shamser.ahmed@lexisnexis.co.uk>
Shamser Ahmed 7 년 전
부모
커밋
36bcd20cb5
6개의 변경된 파일113개의 추가작업 그리고 109개의 파일을 삭제
  1. 2 3
      ecl/hql/hqlgram.hpp
  2. 0 7
      ecl/hql/hqlgram.y
  3. 1 98
      ecl/hql/hqlgram2.cpp
  4. 109 0
      ecl/hqlcpp/hqlttcpp.cpp
  5. 1 0
      testing/regress/ecl/key/loopif.xml
  6. 0 1
      testing/regress/ecl/loopif.ecl

+ 2 - 3
ecl/hql/hqlgram.hpp

@@ -653,8 +653,6 @@ public:
     void addConditionalAssign(const attribute & errpos, IHqlExpression * self, IHqlExpression * leftSelect, IHqlExpression * rightSelect, IHqlExpression * field);
     void addConditionalRowAssign(const attribute & errpos, IHqlExpression * self, IHqlExpression * leftSelect, IHqlExpression * rightSelect, IHqlExpression * record);
     void checkAllAssigned(IHqlExpression * originalRecord, IHqlExpression * unadornedRecord, const attribute &errpos);
-    void checkGrouping(const attribute & errpos, HqlExprArray & parms, IHqlExpression* record, IHqlExpression* groups);
-    void checkGrouping(const attribute & errpos, IHqlExpression * dataset, IHqlExpression* record, IHqlExpression* groups);
     void checkFieldMap(IHqlExpression* map, attribute& errpos);
     IHqlExpression * createDistributeCond(IHqlExpression * left, IHqlExpression * right, const attribute & err, const attribute & seqAttr);
     IHqlExpression * addSideEffects(IHqlExpression * expr);
@@ -1315,5 +1313,6 @@ IHqlExpression *reparseTemplateFunction(IHqlExpression * funcdef, IHqlScope *sco
 extern HQL_API void resetLexerUniqueNames();        // to make regression suite consistent
 extern HQL_API int testHqlInternals();
 extern HQL_API int testReservedWords();
-
+extern HQL_API IHqlExpression * normalizeSelects(IHqlExpression * expr);
+extern HQL_API bool checkGroupExpression(HqlExprArray &groups, IHqlExpression *field);
 #endif

+ 0 - 7
ecl/hql/hqlgram.y

@@ -8885,13 +8885,6 @@ simpleDataSet
                             OwnedHqlExpr attrs;
                             OwnedHqlExpr grouping = parser->processSortList($7, no_usertable, dataset, sortItems, NULL, &attrs);
 
-                            if (grouping && !queryAttributeInList(groupedAtom, attrs))
-                            {
-                                parser->checkGrouping($7, dataset,record,grouping);
-                                if (dataset->getOperator() == no_group && isGrouped(dataset))
-                                    parser->reportWarning(CategoryIgnored, WRN_GROUPINGIGNORED, $3.pos, "Grouping of table input will have no effect, was this intended?");
-                            }
-
                             HqlExprArray args;
                             args.append(*LINK(dataset));
                             args.append(*LINK(record));

+ 1 - 98
ecl/hql/hqlgram2.cpp

@@ -7948,110 +7948,13 @@ public:
 
 
 
-static IHqlExpression * normalizeSelects(IHqlExpression * expr)
+IHqlExpression * normalizeSelects(IHqlExpression * expr)
 {
     QuickSelectNormalizer transformer;
     return transformer.transform(expr);
 }
 
 
-void HqlGram::checkGrouping(const attribute& errpos, HqlExprArray & parms, IHqlExpression* record, IHqlExpression* groups)
-{
-    unsigned reckids = record->numChildren();
-    for (unsigned i = 0; i < reckids; i++)
-    {
-        IHqlExpression *field = record->queryChild(i);
-
-        switch(field->getOperator())
-        {
-        case no_record:
-            checkGrouping(errpos, parms, field, groups);
-            break;
-        case no_ifblock:
-            reportError(ERR_GROUP_BADSELECT, errpos, "IFBLOCKs are not supported inside grouped aggregates");
-            break;
-        case no_field:              
-            {
-                IHqlExpression * rawValue = field->queryChild(0);
-                if (rawValue)
-                {
-                    OwnedHqlExpr value = normalizeSelects(rawValue);
-                    bool ok = checkGroupExpression(parms, value);
-
-                    if (!ok)
-                    {
-                        IIdAtom * id = NULL;
-                        
-                        switch(field->getOperator())
-                        {
-                        case no_select:
-                            id = field->queryChild(1)->queryId();
-                            break;
-                        case no_field:  
-                            id = field->queryId();
-                            break;
-                        default:
-                            id = field->queryId();
-                            break;
-                        }
-
-                        StringBuffer msg("Field ");
-                        if (id)
-                            msg.append("'").append(str(id)).append("' ");
-                        msg.append("in TABLE does not appear to be properly defined by grouping conditions");
-                        reportWarning(CategoryUnexpected, ERR_GROUP_BADSELECT,errpos.pos, "%s", msg.str());
-                    }
-                }
-                else if (field->isDatarow())
-                {
-                    checkGrouping(errpos, parms, field->queryRecord(), groups);
-                }
-                else
-                    throwUnexpected();
-            }
-            break;
-        case no_attr:
-        case no_attr_expr:
-        case no_attr_link:
-            break;
-        default:
-            assertex(false);
-        }
-    }   
-}
-
-
-
-// MORE: how about child dataset?
-void HqlGram::checkGrouping(const attribute & errpos, IHqlExpression * dataset, IHqlExpression* record, IHqlExpression* groups)
-{
-    if (!groups) return;
-    assertex(record->getOperator()==no_record);
-
-    //match should be by structure!!
-    HqlExprArray parms1;
-    HqlExprArray parms;
-    groups->unwindList(parms1, no_sortlist);
-
-    //The expressions need normalizing because the selectors need to be normalized before checking for matches.
-    //The problem is that before the tree is tagged replaceSelector() doesn't work.  So have to use
-    //an approximation instead.
-    ForEachItemIn(idx, parms1)
-    {
-        IHqlExpression * cur = &parms1.item(idx);
-        if (cur->getOperator() == no_field)
-            reportError(ERR_GROUP_BADSELECT, errpos, "cannot use field of result record as a grouping parameter");
-        else
-        {
-            IHqlExpression * mapped = normalizeSelects(cur);
-            parms.append(*mapped);
-        }
-    }
-
-    checkGrouping(errpos, parms, record, groups);
-}
-
-
 void HqlGram::checkConditionalAggregates(IIdAtom * name, IHqlExpression * value, const attribute & errpos)
 {
     if (!value->isGroupAggregateFunction())

+ 109 - 0
ecl/hqlcpp/hqlttcpp.cpp

@@ -42,6 +42,7 @@
 #include "hqlalias.hpp"
 #include "hqlir.hpp"
 #include "hqliproj.hpp"
+#include "hqlgram.hpp"
 
 #define TraceExprPrintLog(x, expr) TOSTRLOG(MCdebugInfo(300), unknownJob, x, (expr)->toString);
 //Following are for code that currently cause problems, but are probably a good idea
@@ -13977,6 +13978,9 @@ public:
             if (callIsActivity(expr))
                 checkEmbedActivity(expr);
             break;
+        case no_usertable:
+            checkGrouping(expr);
+            break;
         }
         QuickHqlTransformer::doAnalyse(expr);
     }
@@ -13986,6 +13990,8 @@ protected:
     void checkJoin(IHqlExpression * expr);
     void checkChoosen(IHqlExpression * expr);
     void checkEmbedActivity(IHqlExpression * expr);
+    void checkGrouping(IHqlExpression * expr);
+    void checkGrouping(HqlExprArray & parms, IHqlExpression * record, IHqlExpression * groups);
     void reportError(int errNo, const char * format, ...) __attribute__((format(printf, 3, 4)));
     void reportWarning(WarnErrorCategory category, int warnNo, const char * format, ...) __attribute__((format(printf, 4, 5)));
 protected:
@@ -14101,6 +14107,109 @@ void SemanticErrorChecker::checkEmbedActivity(IHqlExpression * call)
     }
 }
 
+void SemanticErrorChecker::checkGrouping(IHqlExpression * expr)
+{
+    if (expr->hasAttribute(groupedAtom))
+        return;
+
+    IHqlExpression * groups = queryDatasetGroupBy(expr);
+    if (!groups)
+        return;
+
+    IHqlExpression * ds = expr->queryChild(0);
+    IHqlExpression * record = expr->queryChild(1);
+    assertex(record->getOperator()==no_record);
+
+    if (ds->getOperator() == no_group && isGrouped(ds))
+        reportWarning(CategoryIgnored, WRN_GROUPINGIGNORED, "Grouping of table input will have no effect, was this intended?");
+
+    HqlExprArray parms1;
+    HqlExprArray parms;
+    groups->unwindList(parms1, no_sortlist);
+
+    //The expressions need normalizing because the selectors need to be normalized before checking for matches.
+    //The problem is that before the tree is tagged replaceSelector() doesn't work.  So have to use
+    //an approximation instead.
+    ForEachItemIn(idx, parms1)
+    {
+        IHqlExpression * cur = &parms1.item(idx);
+        if (cur->getOperator() == no_field)
+            reportError(ERR_GROUP_BADSELECT, "cannot use field of result record as a grouping parameter");
+        else
+        {
+            IHqlExpression * mapped = normalizeSelects(cur);
+            parms.append(*mapped);
+        }
+    }
+
+    checkGrouping(parms, record, groups);
+}
+
+void SemanticErrorChecker::checkGrouping(HqlExprArray & parms, IHqlExpression * record, IHqlExpression * groups)
+{
+    unsigned reckids = record->numChildren();
+    for (unsigned i = 0; i < reckids; i++)
+    {
+        IHqlExpression *field = record->queryChild(i);
+
+        switch(field->getOperator())
+        {
+        case no_record:
+            checkGrouping(parms, field, groups);
+            break;
+        case no_ifblock:
+            reportError(ERR_GROUP_BADSELECT, "IFBLOCKs are not supported inside grouped aggregates");
+            return;
+        case no_field:
+            {
+                IHqlExpression * rawValue = field->queryChild(0);
+                if (rawValue)
+                {
+                    OwnedHqlExpr value = normalizeSelects(rawValue);
+                    bool ok = checkGroupExpression(parms, value);
+
+                    if (!ok)
+                    {
+                        IIdAtom * id = NULL;
+
+                        switch(field->getOperator())
+                        {
+                        case no_select:
+                            id = field->queryChild(1)->queryId();
+                            break;
+                        case no_field:
+                            id = field->queryId();
+                            break;
+                        default:
+                            id = field->queryId();
+                            break;
+                        }
+
+                        StringBuffer msg("Field ");
+                        if (id)
+                            msg.append("'").append(str(id)).append("' ");
+                        msg.append("in TABLE does not appear to be properly defined by grouping conditions");
+                        reportWarning(CategoryUnexpected, ERR_GROUP_BADSELECT, "%s", msg.str());
+                    }
+                }
+                else if (field->isDatarow())
+                {
+                    checkGrouping(parms, field->queryRecord(), groups);
+                }
+                else
+                    throwUnexpected();
+            }
+            break;
+        case no_attr:
+        case no_attr_expr:
+        case no_attr_link:
+            break;
+        default:
+            assertex(false);
+        }
+    }
+}
+
 void SemanticErrorChecker::reportError(int errNo, const char * format, ...)
 {
     ECLlocation location;

+ 1 - 0
testing/regress/ecl/key/loopif.xml

@@ -1,3 +1,4 @@
+<Warning><Code>2168</Code><Filename>loopif.ecl</Filename><Line>27</Line><Source>eclcc</Source><Message>Field 'i' in TABLE does not appear to be properly defined by grouping conditions</Message></Warning>
 <Dataset name='Result 1'>
  <Row><i>61</i><j>1</j><c>4</c></Row>
  <Row><i>62</i><j>2</j><c>4</c></Row>

+ 0 - 1
testing/regress/ecl/loopif.ecl

@@ -14,7 +14,6 @@
     See the License for the specific language governing permissions and
     limitations under the License.
 ############################################################################## */
-#onwarning(2168, ignore); // Disabled warning until it is moved out of parser
 
 rec := RECORD
  unsigned4 i;