Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-32054 Prevent global undefined due to out of order evaluation #18779

Merged
merged 1 commit into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ecl/hql/hqltrans.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,8 @@ private:
class HQL_API ConditionalHqlTransformer : public NewHqlTransformer
{
public:
enum { CTFnoteifactions = 0x0001,
enum { CTFnone = 0x0000,
CTFnoteifactions = 0x0001,
CTFnoteifdatasets = 0x0002,
CTFnoteifdatarows = 0x0004,
CTFnoteifall = 0x0008,
Expand Down
24 changes: 16 additions & 8 deletions ecl/hqlcpp/hqlttcpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8315,7 +8315,7 @@ IHqlDataset * queryRootDataset(IHqlExpression * dataset)
//therefore, there is no need to special case if actions. Thor on the other hand will cause it to be executed unnecessarily.
static HqlTransformerInfo newScopeMigrateTransformerInfo("NewScopeMigrateTransformer");
NewScopeMigrateTransformer::NewScopeMigrateTransformer(IWorkUnit * _wu, HqlCppTranslator & _translator)
: HoistingHqlTransformer(newScopeMigrateTransformerInfo, 0), translator(_translator)
: HoistingHqlTransformer(newScopeMigrateTransformerInfo, CTFnone), translator(_translator)
{
wu = _wu;
isRoxie = translator.targetRoxie();
Expand Down Expand Up @@ -8622,7 +8622,7 @@ bool AutoScopeMigrateInfo::doAutoHoist(IHqlExpression * transformed, bool minimi

static HqlTransformerInfo autoScopeMigrateTransformerInfo("AutoScopeMigrateTransformer");
AutoScopeMigrateTransformer::AutoScopeMigrateTransformer(IWorkUnit * _wu, HqlCppTranslator & _translator)
: NewHqlTransformer(autoScopeMigrateTransformerInfo), translator(_translator)
: HoistingHqlTransformer(autoScopeMigrateTransformerInfo, CTFnone), translator(_translator)
{
wu = _wu;
isRoxie = (translator.getTargetClusterType() == RoxieCluster);
Expand All @@ -8631,7 +8631,6 @@ AutoScopeMigrateTransformer::AutoScopeMigrateTransformer(IWorkUnit * _wu, HqlCpp
hasCandidate = false;
activityDepth = 0;
curGraph = 1;
globalTarget = NULL;
}

//Ensure all input activities are marked as never hoisting, but child activities are unaffected
Expand Down Expand Up @@ -8861,7 +8860,7 @@ IHqlExpression * AutoScopeMigrateTransformer::createTransformed(IHqlExpression *
//else hoist it within the current graph, otherwise it can get hoisted before globals on datasets that
//it is dependent on.
if (extra->firstUseIsConditional)
globalTarget->append(*createWrapper(no_thor, setResult.getClear()));
appendToTarget(*createWrapper(no_thor, setResult.getClear()));
else
graphActions.append(*setResult.getClear());
transformed.setown(getResult.getClear());
Expand All @@ -8871,11 +8870,20 @@ IHqlExpression * AutoScopeMigrateTransformer::createTransformed(IHqlExpression *
}


void AutoScopeMigrateTransformer::transformRoot(const HqlExprArray & in, HqlExprArray & out)
IHqlExpression * AutoScopeMigrateTransformer::doTransformIndependent(IHqlExpression * expr)
{
globalTarget = &out;
NewHqlTransformer::transformRoot(in, out);
globalTarget = NULL;
AutoScopeMigrateTransformer transformer(wu, translator);

HqlExprArray exprs;
unwindCommaCompound(exprs, expr);
transformer.analyseArray(exprs, 0);
transformer.analyseArray(exprs, 1);
if (!transformer.worthTransforming())
return LINK(expr);

HqlExprArray results;
transformer.transformRoot(exprs, results);
return createActionList(results);
}


Expand Down
6 changes: 2 additions & 4 deletions ecl/hqlcpp/hqlttcpp.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -729,13 +729,11 @@ public:
bool neverHoist = false;
};

class AutoScopeMigrateTransformer : public NewHqlTransformer
class AutoScopeMigrateTransformer : public HoistingHqlTransformer
{
public:
AutoScopeMigrateTransformer(IWorkUnit * _wu, HqlCppTranslator & _translator);

void transformRoot(const HqlExprArray & in, HqlExprArray & out);

bool worthTransforming() const { return hasCandidate; }

protected:
Expand All @@ -751,6 +749,7 @@ protected:
IHqlExpression * transformCond(IHqlExpression * expr);
void doAnalyseExpr(IHqlExpression * expr);
void doAnalyseConditionalExpr(IHqlExpression * expr, unsigned firstConditional);
virtual IHqlExpression * doTransformIndependent(IHqlExpression * expr) override;

inline AutoScopeMigrateInfo * queryBodyExtra(IHqlExpression * expr) { return static_cast<AutoScopeMigrateInfo *>(queryTransformExtra(expr->queryBody())); }

Expand All @@ -765,7 +764,6 @@ private:
unsigned graphDepth = 0;
HqlExprArray graphActions;
unsigned activityDepth;
HqlExprArray * globalTarget;
};

//---------------------------------------------------------------------------
Expand Down
Loading