More refactoring and cleaning up the code

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

ecl/hql/hqlexpr.cpp

@@ -11507,23 +11507,22 @@ inline IHqlExpression * normalizeSelectLhs(IHqlExpression * lhs, bool & isNew)
     }
 }
 
-inline void checkRhsSelect(IHqlExpression * rhs, IHqlExpression * attr)
+inline void checkRhsSelect(IHqlExpression * rhs)
 {
 #ifdef _DEBUG
     node_operator rhsOp = rhs->getOperator();
-    assertex(rhsOp == no_field || rhsOp == no_ifblock || rhsOp == no_indirect || attr && attr->queryName() == internalAtom);
+    assertex(rhsOp == no_field || rhsOp == no_ifblock || rhsOp == no_indirect);
 #endif
 }
 
-extern IHqlExpression * createSelectExpr(IHqlExpression * _lhs, IHqlExpression * rhs, IHqlExpression * _attr)
+extern IHqlExpression * createSelectExpr(IHqlExpression * _lhs, IHqlExpression * rhs, bool _isNew)
 {
     OwnedHqlExpr lhs = _lhs;
-    OwnedHqlExpr attr = _attr;
-    bool isNew = (attr != NULL);
+    bool isNew = _isNew;
     IHqlExpression * normalLhs = normalizeSelectLhs(lhs, isNew);
     IHqlExpression * newAttr = isNew ? newSelectAttrExpr : NULL;
 
-    checkRhsSelect(rhs, attr);
+    checkRhsSelect(rhs);
 
     type_t t = rhs->queryType()->getTypeCode();
     if (t == type_table || t == type_groupedtable)
@@ -11535,39 +11534,6 @@ extern IHqlExpression * createSelectExpr(IHqlExpression * _lhs, IHqlExpression *
     return ret->closeExpr();
 }
 
-extern IHqlExpression * createSelectExpr(IHqlExpression * _lhs, IHqlExpression * rhs)
-{
-    OwnedHqlExpr lhs = _lhs;
-    bool isNew = false;
-    IHqlExpression * normalLhs = normalizeSelectLhs(lhs, isNew);
-    IHqlExpression * newAttr = isNew ? newSelectAttrExpr : NULL;
-
-    checkRhsSelect(rhs, NULL);
-
-    type_t t = rhs->queryType()->getTypeCode();
-    if (t == type_table || t == type_groupedtable)
-        return createDataset(no_select, LINK(normalLhs), createComma(rhs, LINK(newAttr)));
-    if (t == type_row)
-        return createRow(no_select, LINK(normalLhs), createComma(rhs, LINK(newAttr)));
-
-    IHqlExpression * ret = new CHqlSelectExpression(LINK(normalLhs), rhs, LINK(newAttr));
-    return ret->closeExpr();
-}
-
-extern IHqlExpression * createNewSelectExpr(IHqlExpression * lhs, IHqlExpression * rhs)
-{
-    switch (lhs->getOperator())
-    {
-    case no_left:
-    case no_right:
-    case no_top:
-    case no_activetable:
-        //Minor special cases
-        return createSelectExpr(lhs, rhs, NULL);
-    }
-    return createSelectExpr(lhs, rhs, LINK(newSelectAttrExpr));
-}
-
 extern IHqlExpression * createSelectExpr(HqlExprArray & args)
 {
     IHqlExpression * lhs = &args.item(0);
@@ -11589,8 +11555,7 @@ extern IHqlExpression * createSelectExpr(HqlExprArray & args)
         args.append(*LINK(newSelectAttrExpr));
 
     IHqlExpression * rhs = &args.item(1);
-    IHqlExpression * attr = args.ordinality() > 2 ? &args.item(2) : NULL;
-    checkRhsSelect(rhs, attr);
+    checkRhsSelect(rhs);
 
     type_t t = rhs->queryType()->getTypeCode();
     if (t == type_table || t == type_groupedtable)
@@ -13284,13 +13249,13 @@ IHqlExpression * replaceSelectorDataset(IHqlExpression * expr, IHqlExpression *
 {
     assertex(expr->getOperator() == no_select);
     IHqlExpression * ds = expr->queryChild(0);
-    IHqlExpression * newSelector;
     if ((ds->getOperator() == no_select) && !ds->isDataset())
-        newSelector = replaceSelectorDataset(ds, newDataset);
+    {
+        OwnedHqlExpr newSelector = replaceSelectorDataset(ds, newDataset);
+        return replaceChild(expr, 0, newSelector);
+    }
     else
-        newSelector = LINK(newDataset);
-    //Call createSelectExpr instead of clone() so that no_newrow/no_activerow are handled correctly
-    return createSelectExpr(newSelector, LINK(expr->queryChild(1)), LINK(expr->queryChild(2)));
+        return replaceChild(expr, 0, newDataset);
 }
 
 IHqlExpression * querySkipDatasetMeta(IHqlExpression * dataset)

ecl/hql/hqlexpr.hpp

@@ -1325,10 +1325,10 @@ extern HQL_API IHqlExpression* createValue(node_operator op, HqlExprArray& opera
 extern HQL_API IHqlExpression *createValue(node_operator op, IHqlExpression *p1);
 extern HQL_API IHqlExpression* createConstant(int ival);
 extern HQL_API IHqlExpression* createBoolExpr(node_operator op, HqlExprArray& operands);
-extern HQL_API IHqlExpression* createSelectExpr(IHqlExpression * lhs, IHqlExpression * rhs, IHqlExpression * attr);
-extern HQL_API IHqlExpression* createSelectExpr(IHqlExpression * lhs, IHqlExpression * rhs);
+extern HQL_API IHqlExpression* createSelectExpr(IHqlExpression * lhs, IHqlExpression * rhs, bool isNew);
+inline IHqlExpression* createSelectExpr(IHqlExpression * lhs, IHqlExpression * rhs) { return createSelectExpr(lhs, rhs, false); }
+inline IHqlExpression* createNewSelectExpr(IHqlExpression * lhs, IHqlExpression * rhs) { return createSelectExpr(lhs, rhs, true); }
 extern HQL_API IHqlExpression* createSelectExpr(HqlExprArray & args);
-extern HQL_API IHqlExpression* createNewSelectExpr(IHqlExpression * lhs, IHqlExpression * rhs);
 
 inline IHqlExpression* createAction(node_operator op, HqlExprArray& operands) { return createValue(op, makeVoidType(), operands); }
 

ecl/hql/hqlpmap.cpp

@@ -52,10 +52,10 @@ static IHqlExpression * optimizedReplaceSelector(IHqlExpression * expr, IHqlExpr
         {
             IHqlExpression * lhs = expr->queryChild(0);
             IHqlExpression * field = expr->queryChild(1);
-            IHqlExpression * newLhs;
+            OwnedHqlExpr newLhs;
             if (expr->hasProperty(newAtom))
             {
-                newLhs = optimizedReplaceSelector(lhs, oldDataset, newDataset);
+                newLhs.setown(optimizedReplaceSelector(lhs, oldDataset, newDataset));
             }
             else
             {
@@ -70,11 +70,11 @@ static IHqlExpression * optimizedReplaceSelector(IHqlExpression * expr, IHqlExpr
                     return createSelectExpr(LINK(newDataset->queryNormalizedSelector()), LINK(field));
                 }
                 else
-                    newLhs = optimizedReplaceSelector(lhs, oldDataset, newDataset);
+                    newLhs.setown(optimizedReplaceSelector(lhs, oldDataset, newDataset));
             }
 
             if (newLhs)
-                return createSelectExpr(newLhs, LINK(field), LINK(expr->queryChild(2)));
+                return replaceChild(expr, 0, newLhs);
             return NULL;
         }
     case no_implicitcast:

ecl/hql/hqlutil.cpp

@@ -1140,8 +1140,16 @@ IHqlExpression * replaceChild(IHqlExpression * expr, unsigned childIndex, IHqlEx
     if (oldChild == newChild)
         return LINK(expr);
     HqlExprArray args;
-    unwindChildren(args, expr);
-    args.replace(*LINK(newChild), childIndex);
+    if (childIndex == 0)
+    {
+        args.append(*LINK(newChild));
+        unwindChildren(args, expr, 1);
+    }
+    else
+    {
+        unwindChildren(args, expr);
+        args.replace(*LINK(newChild), childIndex);
+    }
     return expr->clone(args);
 }
 

ecl/hqlcpp/hqlcpp.ipp

@@ -759,7 +759,6 @@ class HQLCPP_API HqlCppTranslator : public CInterface, implements IHqlCppTransla
     friend class FetchBuilder;
     friend class MonitorExtractor;
     friend class NlpParseContext;
-    friend class FilterExtractor;
     friend class KeyedJoinInfo;
     friend class ChildGraphBuilder;
 public:

ecl/hqlcpp/hqlhtcpp.cpp

@@ -7341,178 +7341,6 @@ public:
     CHqlBoundTarget     value;
 };
 
-static HqlTransformerInfo filterExtractorInfo("FilterExtractor");
-class FilterExtractor : public NewHqlTransformer
-{
-public:
-    FilterExtractor(HqlCppTranslator & _translator, BuildCtx & _classctx, BuildCtx & _ctx, IHqlExpression * _rootTable) 
-        : NewHqlTransformer(filterExtractorInfo), translator(_translator), classctx(_classctx), ctx(_ctx) { rootTable.set(_rootTable->queryNormalizedSelector()); }
-
-    void assignCursors(IHqlExpression * helper);
-    IHqlExpression * buildExtractLookupFields(IHqlExpression * expr);
-    IHqlExpression * createTransformed(IHqlExpression * expr);
-
-protected:
-    struct AssignPairs
-    {
-        HqlExprArray from;
-        CIArrayOf<CHqlBoundTargetItem> to;
-    };
-
-    void addTable(IHqlExpression * expr);
-    IHqlExpression * extractExpr(IHqlExpression * expr, AssignPairs & assigns);
-    IHqlExpression * querySelectorTable(IHqlExpression * expr);
-
-
-public:
-    HqlCppTranslator & translator;
-    BuildCtx & classctx;
-    BuildCtx & ctx;
-    CIArray tables;
-    AssignPairs  ctxAssign;
-    HqlExprArray ctxAssignDirectFrom;
-    HqlExprArray ctxAssignDirectTo;
-    AssignPairs  extractAssign;
-    HqlExprAttr rootTable;
-};
-
-void FilterExtractor::addTable(IHqlExpression * table)
-{
-    if (table == rootTable)
-        return;
-
-    BoundRow * cursor;
-    if (table)
-        cursor = static_cast<BoundRow *>(ctx.queryAssociation(table, AssocCursor, NULL));
-    else
-        cursor = (BoundRow *)ctx.queryFirstAssociation(AssocCursor);
-
-    assertex(cursor);
-    if (tables.find(*cursor) == NotFound)
-    {
-        StringBuffer newCursorName;
-        translator.getUniqueId(newCursorName.append("cur"));
-        BoundRow * newCursor = translator.bindTableCursor(classctx, cursor->queryDataset(), newCursorName, cursor->querySide(), cursor->querySelSeq());
-
-        IHqlExpression * oldVar = cursor->queryBound();
-        IHqlExpression * newVar = newCursor->queryBound();
-        ctxAssignDirectFrom.append(*LINK(oldVar));
-        ctxAssignDirectTo.append(*LINK(newVar));
-        tables.append(*LINK(cursor));
-
-        StringBuffer s;
-        s.append("const unsigned char * ").append(newCursorName).append(";");
-        classctx.addQuoted(s);
-    }
-}
-
-IHqlExpression * FilterExtractor::extractExpr(IHqlExpression * expr, AssignPairs & assigns)
-{
-    unsigned match = assigns.from.find(*expr);
-    if (match == NotFound)
-    {
-        CHqlBoundTargetItem & target = * new CHqlBoundTargetItem;
-        translator.createTempFor(classctx, expr, target.value);
-        assigns.from.append(*LINK(expr));
-        assigns.to.append(target);
-        translator.ensureSerialized(classctx, target.value);
-        match = assigns.to.ordinality()-1;
-    }
-
-    return assigns.to.item(match).value.getTranslatedExpr();
-}
-
-IHqlExpression * FilterExtractor::querySelectorTable(IHqlExpression * expr)
-{
-    loop
-    {
-        switch (expr->getOperator())
-        {
-        case no_select:
-            expr = expr->queryChild(0);
-            break;
-        case no_left:
-        case no_right:
-        case no_self:
-        case no_top:
-        case no_activetable:
-            return expr;
-        default:
-            if (expr->isDataset() || expr->isDatarow())
-                return expr->queryNormalizedSelector();
-            return NULL;
-        }
-    }
-}
-
-IHqlExpression * FilterExtractor::createTransformed(IHqlExpression * expr)
-{
-    switch (expr->getOperator())
-    {
-    case no_field:
-        return LINK(expr);
-    case no_select:
-        {
-            assertex(!expr->hasProperty(newAtom));
-
-            if (querySelectorTable(expr) == rootTable)
-                return LINK(expr);
-
-            addTable(querySelectorTable(expr));
-            return extractExpr(expr, extractAssign);
-        }
-    case no_counter:
-        return extractExpr(expr, ctxAssign);
-    }
-    return NewHqlTransformer::createTransformed(expr);
-}
-
-
-IHqlExpression * FilterExtractor::buildExtractLookupFields(IHqlExpression * expr)
-{
-    if (expr->getOperator() != no_filter)
-        return LINK(expr);
-
-    HqlExprArray args;
-    ForEachChild(idx, expr)
-    {
-        IHqlExpression * cur = expr->queryChild(idx);
-        if (idx == 0)
-            args.append(*LINK(cur));
-        else
-            args.append(*createTransformed(cur));
-    }
-
-    if ((ctxAssign.from.ordinality() == 0) && (ctxAssignDirectFrom.ordinality() == 0))
-        return LINK(expr);
-
-    //virtual void extractLookupFields() {}
-    BuildCtx funcctx(classctx);
-    funcctx.addQuotedCompound("virtual void extractLookupFields()");
-    ForEachItemIn(idx2, extractAssign.from)
-        translator.buildExprAssign(funcctx, extractAssign.to.item(idx2).value, &extractAssign.from.item(idx2));
-    return expr->clone(args);
-}
-
-
-void FilterExtractor::assignCursors(IHqlExpression * helper)
-{
-    ForEachItemIn(idx1, ctxAssignDirectFrom)
-    {
-        OwnedHqlExpr target = createSelectExpr(LINK(helper), LINK(&ctxAssignDirectTo.item(idx1)), createAttribute(internalAtom));
-        ctx.addAssign(target, &ctxAssignDirectFrom.item(idx1));
-    }
-    ForEachItemIn(idx2, ctxAssign.from)
-    {
-        CHqlBoundTarget target;
-        target.set(ctxAssign.to.item(idx2).value);
-        if (target.length)      target.length.setown(createSelectExpr(LINK(helper), LINK(target.length), createAttribute(internalAtom)));
-        if (target.expr)        target.expr.setown(createSelectExpr(LINK(helper), LINK(target.expr), createAttribute(internalAtom)));
-        translator.buildExprAssign(ctx, target, &ctxAssign.from.item(idx2));
-    }
-}
-
-
 void HqlCppTranslator::doBuildExprCountFile(BuildCtx & ctx, IHqlExpression * _expr, CHqlBoundExpr & tgt)
 {
     //NB: This is currently only applied to fixed width unfiltered files

ecl/hqlcpp/hqlttcpp.cpp

@@ -2737,19 +2737,7 @@ IHqlExpression * ThorHqlTransformer::normalizeScalarAggregate(IHqlExpression * e
     if (!project)
         throwUnexpected();
     IHqlExpression * field = project->queryRecord()->queryChild(0);
-
-    HqlExprArray args;
-    ForEachChild(i, expr)
-    {
-        IHqlExpression * cur = expr->queryChild(i);
-        if (cur->isAttribute())
-        {
-            _ATOM name = cur->queryName();
-            if ((name != keyedAtom) && (name != prefetchAtom))
-                args.append(*LINK(cur));
-        }
-    }
-    OwnedHqlExpr ret = createSelectExpr(project.getClear(), LINK(field), createExprAttribute(newAtom, args));
+    OwnedHqlExpr ret = createNewSelectExpr(project.getClear(), LINK(field));
     return expr->cloneAllAnnotations(ret);
 }
 
@@ -8742,10 +8730,7 @@ IHqlExpression * HqlScopeTagger::transformSelect(IHqlExpression * expr)
     }
     //MORE: What about child datasets - should really be tagged as
     //if (!isNewDataset && field->isDataset() && !containsSelf(ds) && !isDatasetActive(ds)) isNew = true;
-
-    HqlExprArray attrs;
-    endTableScope(attrs, ds, newDs);
-    return createSelectExpr(newDs.getClear(), LINK(field), createExprAttribute(newAtom, attrs));
+    return createNewSelectExpr(newDs.getClear(), LINK(field));
 }
 
 IHqlExpression * HqlScopeTagger::transformSelectorsAttr(IHqlExpression * expr)
@@ -8925,7 +8910,7 @@ IHqlExpression * HqlScopeTagger::transformWithin(IHqlExpression * dataset, IHqlE
         return createSelectExpr(newDs.getClear(), LINK(field));
     }
     OwnedHqlExpr newDs = transformWithin(ds, scope);
-    return createSelectExpr(newDs.getClear(), LINK(field), createExprAttribute(newAtom));
+    return createNewSelectExpr(newDs.getClear(), LINK(field));
 }
 
 IHqlExpression * HqlScopeTagger::transformRelated(IHqlExpression * expr)

ecl/regress/sqfilt5.ecl

@@ -0,0 +1,44 @@
+/*##############################################################################
+
+    Copyright (C) 2011 HPCC Systems.
+
+    All rights reserved. This program is free software: you can redistribute it and/or modify
+    it under the terms of the GNU Affero General Public License as
+    published by the Free Software Foundation, either version 3 of the
+    License, or (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU Affero General Public License for more details.
+
+    You should have received a copy of the GNU Affero General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+############################################################################## */
+
+import sq;
+sq.DeclareCommon();
+
+#option ('childQueries', true);
+
+// Test filtering at different levels, making sure parent fields are available in the child query.
+// Also tests scoping of sub expressions using within.
+
+udecimal8 todaysDate := 20040602D;
+unsigned4 age(udecimal8 dob) := ((todaysDate - dob) / 10000D);
+
+//MORE: books[1] ave(books)
+
+// Different child operators, all inline.
+house := sqHousePersonBookDs.persons;
+persons := sqHousePersonBookDs.persons;
+books := persons.books;
+
+booksDs := sqBookDs(personid = persons.id);
+personsDs := sqPersonDs(houseid = sqHousePersonBookDs.id);
+booksDsDs := sqBookDs(personid = personsDs.id);
+personsDsDs := sqPersonDs(houseid = sqHouseDs.id);
+booksDsDsDs := sqBookDs(personid = personsDsDs.id);
+
+//people with a book worth more than the rest of their books.
+output(sqHousePersonBookDs.persons, { exists(books(price > sum(__alias__(books)(id != books.id), price))); }, named('NumPeopleExceedBookLimit'));