Skip to content

Commit b65e777

Browse files
committed
review fixes
1 parent 8f4a282 commit b65e777

File tree

9 files changed

+28
-20
lines changed

9 files changed

+28
-20
lines changed

ydb/library/yql/dq/tasks/dq_task_program.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class TSpillingTransformProvider {
1717
TSpillingTransformProvider(const TSpillingSettings& spillingSettings): SpillingSettings(spillingSettings){};
1818

1919
TCallableVisitFunc operator()(TInternName name) {
20-
if (RuntimeVersion >= 50U && SpillingSettings.EnableSpillingInGraceJoin && (name == "GraceJoin" || name == "GraceSelfJoin")) {
20+
if (RuntimeVersion >= 50U && SpillingSettings.IsGraceJoinSpillingEnabled() && (name == "GraceJoin" || name == "GraceSelfJoin")) {
2121
return [name](NKikimr::NMiniKQL::TCallable& callable, const TTypeEnvironment& env) {
2222
TCallableBuilder callableBuilder(env,
2323
TStringBuilder() << callable.GetType()->GetName() << "WithSpilling",

ydb/library/yql/dq/tasks/dq_task_program.h

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,25 @@
77
#include <ydb/library/yql/providers/common/mkql/yql_provider_mkql.h>
88
#include <ydb/library/yql/providers/common/provider/yql_provider.h>
99

10+
#include <ydb/library/yql/providers/dq/common/yql_dq_settings.h>
11+
1012
namespace NYql::NDq {
1113

12-
struct TSpillingSettings {
14+
class TSpillingSettings {
15+
public:
16+
TSpillingSettings() = default;
17+
explicit TSpillingSettings(ui64 mask) : Mask(mask) {};
18+
1319
operator bool() const {
14-
return EnableSpillingInGraceJoin;
20+
return Mask;
21+
}
22+
23+
bool IsGraceJoinSpillingEnabled() const {
24+
return Mask & ui64(TDqConfiguration::EEnabledSpillingNodes::GraceJoin);
1525
}
16-
17-
bool EnableSpillingInGraceJoin = false;
26+
27+
private:
28+
const ui64 Mask = 0;
1829
};
1930

2031
const TStructExprType* CollectParameters(NNodes::TCoLambda program, TExprContext& ctx);

ydb/library/yql/minikql/comp_nodes/mkql_grace_join.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,13 +1171,13 @@ IComputationNode* WrapGraceJoinCommon(TCallable& callable, const TComputationNod
11711171
IComputationNode* WrapGraceJoin(TCallable& callable, const TComputationNodeFactoryContext& ctx) {
11721172
MKQL_ENSURE(callable.GetInputsCount() == 8, "Expected 8 args");
11731173

1174-
return WrapGraceJoinCommon(callable, ctx, false, callable.IsSpillingSupported());
1174+
return WrapGraceJoinCommon(callable, ctx, false, HasSpillingFlag(callable));
11751175
}
11761176

11771177
IComputationNode* WrapGraceSelfJoin(TCallable& callable, const TComputationNodeFactoryContext& ctx) {
11781178
MKQL_ENSURE(callable.GetInputsCount() == 7, "Expected 7 args");
11791179

1180-
return WrapGraceJoinCommon(callable, ctx, true, callable.IsSpillingSupported());
1180+
return WrapGraceJoinCommon(callable, ctx, true, HasSpillingFlag(callable));
11811181
}
11821182

11831183
}

ydb/library/yql/minikql/mkql_node.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,10 +1075,6 @@ friend class TNode;
10751075
UniqueId = uniqueId;
10761076
}
10771077

1078-
bool IsSpillingSupported() const {
1079-
return TStringBuf(GetType()->GetName()).EndsWith("WithSpilling"_sb);
1080-
}
1081-
10821078
private:
10831079
TCallable(ui32 inputsCount, TRuntimeNode* inputs, TCallableType* type, bool validate = true);
10841080
TCallable(TRuntimeNode result, TCallableType* type, bool validate = true);

ydb/library/yql/minikql/mkql_program_builder.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ inline void AddAnyJoinSide(EAnyJoinSettings& combined, EAnyJoinSettings value) {
8686
combined = (EAnyJoinSettings)combinedVal;
8787
}
8888

89+
inline bool HasSpillingFlag(const TCallable& callable) {
90+
return TStringBuf(callable.GetType()->GetName()).EndsWith("WithSpilling"_sb);
91+
}
92+
8993
#define MKQL_SCRIPT_TYPES(xx) \
9094
xx(Unknown, 0, unknown, false) \
9195
xx(Python, 1, python, false) \

ydb/library/yql/providers/dq/common/yql_dq_settings.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,6 @@ TDqConfiguration::TDqConfiguration() {
109109
throw yexception() << "Empty value item";
110110
}
111111
auto value = FromString<EEnabledSpillingNodes>(s);
112-
if (value == EEnabledSpillingNodes::All) {
113-
return ui64(-1);
114-
}
115112
res |= ui64(value);
116113
}
117114
return res;

ydb/library/yql/providers/dq/common/yql_dq_settings.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct TDqSettings {
3030

3131
enum class EEnabledSpillingNodes : ui64 {
3232
GraceJoin = 1ULL /* "GraceJoin" */,
33-
All = 64ULL /* "All" */,
33+
All = ~0ULL /* "All" */,
3434
};
3535

3636
struct TDefault {
@@ -223,9 +223,9 @@ struct TDqSettings {
223223
return SpillingEngine.Get().GetOrElse(TDqSettings::TDefault::SpillingEngine) != ESpillingEngine::Disable;
224224
}
225225

226-
bool IsSpillingInGraceJoinEnabled() const {
227-
ui64 mask = EnableSpillingNodes.Get().GetOrElse(0);
228-
return IsSpillingEnabled() && (mask & ui64(EEnabledSpillingNodes::GraceJoin));
226+
ui64 GetEnabledSpillingNodes() const {
227+
if (!IsSpillingEnabled()) return 0;
228+
return EnableSpillingNodes.Get().GetOrElse(0);
229229
}
230230

231231
bool IsDqReplicateEnabled(const TTypeAnnotationContext& typesCtx) const {

ydb/library/yql/providers/dq/planner/execution_planner.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ namespace NYql::NDqs {
676676
Y_ABORT_UNLESS(false);
677677
}
678678
*/
679-
TSpillingSettings spillingSettings{Settings->IsSpillingInGraceJoinEnabled()};
679+
TSpillingSettings spillingSettings{Settings->GetEnabledSpillingNodes()};
680680
StagePrograms[stageInfo.first] = std::make_tuple(
681681
NDq::BuildProgram(
682682
stage.Program(), *paramsType, compiler, typeEnv, *FunctionRegistry,

ydb/library/yql/providers/dq/provider/exec/yql_dq_exectransformer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,7 @@ class TDqExecTransformer: public TExecTransformerBase, TCounters
761761

762762
TVector<TExprBase> fakeReads;
763763
auto paramsType = NDq::CollectParameters(programLambda, ctx);
764-
NDq::TSpillingSettings spillingSettings{State->Settings->IsSpillingInGraceJoinEnabled()};
764+
NDq::TSpillingSettings spillingSettings{State->Settings->GetEnabledSpillingNodes()};
765765
*lambda = NDq::BuildProgram(
766766
programLambda, *paramsType, compiler, typeEnv, *State->FunctionRegistry,
767767
ctx, fakeReads, spillingSettings);

0 commit comments

Comments
 (0)