Common up null datasets with minor differences #2107

Merged
merged 1 commit into from Apr 18, 2012

Projects

None yet

3 participants

@ghalliday
Member

A minor improvement to the generated code for a few queries.
The type of a dataset may contain an annotation indicating the name of
the original record. If one null dataset expression contains an
annotation and another doesn't they should be treated as identical.

(Had a minor effect on a couple of queries).

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

@ghalliday
Member

@rengolin Something to check.

@rengolin rengolin and 2 others commented on an outdated diff Apr 16, 2012
ecl/hql/hqlfold.cpp
@@ -3950,6 +3950,15 @@ static bool isWorthPercolating(const HqlExprArray & exprs)
}
+bool constantsMatch(IHqlExpression * left, IHqlExpression * right)
+{
+ if (left->queryBody() == right->queryBody())
+ return true;
+ if ((left->getOperator() != no_null) || (right->getOperator() != no_null))
+ return false;
+ return recordTypesMatch(left, right);
@rengolin
rengolin Apr 16, 2012 Contributor

If this gets used by non-constants (by accident), just comparing the record types might produce wrong results. Shouldn't we assert if the expressions are not constants? Or at least restrict the pointer type to constant expressions.

@ghalliday
ghalliday Apr 16, 2012 Member

This works equally well on non constants. I could call the function expressionsEquivalent() instead, but it is only currently used on constants.

@richardkchapman
richardkchapman Apr 16, 2012 Member

I think would have been clearer as

   if ((left->getOperator() == no_null) && (right->getOperator() == no_null))
    return recordTypesMatch(left, right);
   else
     return false;
@rengolin
rengolin Apr 16, 2012 Contributor

Not sure it's the same thing... In Gavin's code, if neither are null, it still checks the types.

But I believe Richard's code is more correct, since if they're not null, and their bodies aren't equal, they won't match.

@ghalliday if it can be used by non-constants, I think it should have a more generic name...

@ghalliday
Member

Agreed - try again.

@rengolin
Contributor

Looks good.

@ghalliday ghalliday Common up null datasets with minor differences
A minor improvement to the generated code for a few queries.
The type of a dataset may contain an annotation indicating the name of
the original record.  If one null dataset expression contains an
annotation and another doesn't they should be treated as identical.

(Had a minor effect on a couple of queries).

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
de332a3
@richardkchapman richardkchapman merged commit 1dfd45f into hpcc-systems:master Apr 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment