Skip to content

Commit

Permalink
KIKIMR-21018: always allow pickling in AggregateExpander (ydb-platfor…
Browse files Browse the repository at this point in the history
  • Loading branch information
qrort authored Feb 15, 2024
1 parent 06c0c2c commit 96288e5
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 13 deletions.
2 changes: 1 addition & 1 deletion ydb/library/yql/core/yql_aggregate_expander.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ bool TAggregateExpander::IsNeedPickle(const TVector<const TTypeAnnotationNode*>&
{
bool needPickle = false;
for (auto type : keyItemTypes) {
needPickle = needPickle || AllowPickle && !IsDataOrOptionalOfData(type);
needPickle |= !IsDataOrOptionalOfData(type);
}
return needPickle;
}
Expand Down
6 changes: 2 additions & 4 deletions ydb/library/yql/core/yql_aggregate_expander.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ namespace NYql {

class TAggregateExpander {
public:
TAggregateExpander(bool allowPickle, bool usePartitionsByKeys, const bool useFinalizeByKeys, const TExprNode::TPtr& node, TExprContext& ctx, TTypeAnnotationContext& typesCtx,
TAggregateExpander(bool usePartitionsByKeys, const bool useFinalizeByKeys, const TExprNode::TPtr& node, TExprContext& ctx, TTypeAnnotationContext& typesCtx,
bool forceCompact = false, bool compactForDistinct = false, bool usePhases = false)
: Node(node)
, Ctx(ctx)
, TypesCtx(typesCtx)
, AllowPickle(allowPickle)
, UsePartitionsByKeys(usePartitionsByKeys)
, UseFinalizeByKeys(useFinalizeByKeys)
, ForceCompact(forceCompact)
Expand Down Expand Up @@ -93,7 +92,6 @@ class TAggregateExpander {
const TExprNode::TPtr Node;
TExprContext& Ctx;
TTypeAnnotationContext& TypesCtx;
bool AllowPickle;
bool UsePartitionsByKeys;
bool UseFinalizeByKeys = false;
bool ForceCompact;
Expand Down Expand Up @@ -132,7 +130,7 @@ class TAggregateExpander {
};

inline TExprNode::TPtr ExpandAggregatePeepholeImpl(const TExprNode::TPtr& node, TExprContext& ctx, TTypeAnnotationContext& typesCtx, const bool useFinalizeByKey, const bool useBlocks) {
TAggregateExpander aggExpander(false, !useFinalizeByKey && !useBlocks, useFinalizeByKey, node, ctx, typesCtx, true);
TAggregateExpander aggExpander(!useFinalizeByKey && !useBlocks, useFinalizeByKey, node, ctx, typesCtx, true);
return aggExpander.ExpandAggregate();
}

Expand Down
13 changes: 7 additions & 6 deletions ydb/library/yql/core/yql_expr_constraint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2654,9 +2654,10 @@ class TCallableConstraintTransformer : public TCallableTransformerBase<TCallable
}

template<bool Wide>
static TPartOfConstraintBase::TSetType GetSimpleKeys(const TExprNode& body, const TExprNode::TChildrenType& args, TExprContext& ctx) {
static TPartOfConstraintBase::TSetType GetSimpleKeys(const TExprNode& node, const TExprNode::TChildrenType& args, TExprContext& ctx) {
TPartOfConstraintBase::TSetType keys;
if (body.IsCallable("AggrNotEquals")) {
if (node.IsCallable("AggrNotEquals")) {
const TExprNode& body = node.Head().IsCallable("StablePickle") ? node.Head() : node;
if (body.Head().IsList() && body.Tail().IsList() && body.Head().ChildrenSize() == body.Tail().ChildrenSize()) {
keys.reserve(body.Tail().ChildrenSize());
for (auto i = 0U; i < body.Head().ChildrenSize(); ++i){
Expand All @@ -2679,10 +2680,10 @@ class TCallableConstraintTransformer : public TCallableTransformerBase<TCallable
keys.insert_unique(r->first);
}
}
} else if (body.IsCallable("Or")) {
keys.reserve(body.ChildrenSize());
for (auto i = 0U; i < body.ChildrenSize(); ++i) {
const auto& part = GetSimpleKeys<Wide>(*body.Child(i), args, ctx);
} else if (node.IsCallable("Or")) {
keys.reserve(node.ChildrenSize());
for (auto i = 0U; i < node.ChildrenSize(); ++i) {
const auto& part = GetSimpleKeys<Wide>(*node.Child(i), args, ctx);
keys.insert_unique(part.cbegin(), part.cend());
}
}
Expand Down
4 changes: 4 additions & 0 deletions ydb/library/yql/core/yql_opt_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,8 @@ std::optional<std::pair<TPartOfConstraintBase::TPathType, ui32>> GetPathToKey(co
} else if (body.IsCallable({"CastStruct","FilterMembers"})) {
return GetPathToKey(body.Head(), args);
}
} else if (body.IsCallable("StablePickle")) {
return GetPathToKey(body.Head(), args);
}

return std::nullopt;
Expand Down Expand Up @@ -2054,6 +2056,8 @@ std::optional<TPartOfConstraintBase::TPathType> GetPathToKey(const TExprNode& bo
return GetPathToKey(body.Head().Tail().Head(), arg);
if (IsTransparentIfPresent(body) && &body.Head() == &arg)
return GetPathToKey(body.Child(1)->Tail().Head(), body.Child(1)->Head().Head());
if (body.IsCallable("StablePickle"))
return GetPathToKey(body.Head(), arg);

return std::nullopt;
}
Expand Down
2 changes: 1 addition & 1 deletion ydb/library/yql/dq/opt/dq_opt_log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ TExprBase DqRewriteAggregate(TExprBase node, TExprContext& ctx, TTypeAnnotationC
if (!node.Maybe<TCoAggregateBase>()) {
return node;
}
TAggregateExpander aggExpander(true, !typesCtx.IsBlockEngineEnabled() && !useFinalizeByKey, useFinalizeByKey, node.Ptr(), ctx, typesCtx, false, compactForDistinct, usePhases);
TAggregateExpander aggExpander(!typesCtx.IsBlockEngineEnabled() && !useFinalizeByKey, useFinalizeByKey, node.Ptr(), ctx, typesCtx, false, compactForDistinct, usePhases);
auto result = aggExpander.ExpandAggregate();
YQL_ENSURE(result);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ class TYtLogicalOptProposalTransformer : public TOptimizeTransformerBase {

auto usePhases = State_->Configuration->UseAggPhases.Get().GetOrElse(false);
auto usePartitionsByKeys = State_->Configuration->UsePartitionsByKeysForFinalAgg.Get().GetOrElse(true);
TAggregateExpander aggExpander(true, usePartitionsByKeys, false, node.Ptr(), ctx, *State_->Types, false, false, usePhases);
TAggregateExpander aggExpander(usePartitionsByKeys, false, node.Ptr(), ctx, *State_->Types, false, false, usePhases);
return aggExpander.ExpandAggregate();
}

Expand Down

0 comments on commit 96288e5

Please sign in to comment.