WangGuangxin commented on code in PR #5626:
URL: https://github.com/apache/incubator-gluten/pull/5626#discussion_r1610990812
##########
cpp/velox/substrait/SubstraitToVeloxPlan.cc:
##########
@@ -793,23 +794,34 @@ const core::WindowNode::Frame createWindowFrame(
VELOX_FAIL("the window type only support ROWS and RANGE, and the input
type is ", std::to_string(type));
}
- auto boundTypeConversion = [](::substrait::Expression_WindowFunction_Bound
boundType)
+ auto boundTypeConversion = [&](::substrait::Expression_WindowFunction_Bound
boundType)
-> std::tuple<core::WindowNode::BoundType, core::TypedExprPtr> {
- // TODO: support non-literal expression.
if (boundType.has_current_row()) {
return std::make_tuple(core::WindowNode::BoundType::kCurrentRow,
nullptr);
} else if (boundType.has_unbounded_following()) {
return std::make_tuple(core::WindowNode::BoundType::kUnboundedFollowing,
nullptr);
} else if (boundType.has_unbounded_preceding()) {
return std::make_tuple(core::WindowNode::BoundType::kUnboundedPreceding,
nullptr);
} else if (boundType.has_following()) {
- return std::make_tuple(
- core::WindowNode::BoundType::kFollowing,
- std::make_shared<core::ConstantTypedExpr>(BIGINT(),
variant(boundType.following().offset())));
+ if (boundType.following().has_offset()) {
+ return std::make_tuple(
+ core::WindowNode::BoundType::kFollowing,
+ std::make_shared<core::ConstantTypedExpr>(BIGINT(),
variant(boundType.following().offset())));
+ } else {
+ return std::make_tuple(
+ core::WindowNode::BoundType::kFollowing,
+ exprConverter_->toVeloxExpr(boundType.following().ref(),
inputType));
+ }
} else if (boundType.has_preceding()) {
- return std::make_tuple(
- core::WindowNode::BoundType::kPreceding,
- std::make_shared<core::ConstantTypedExpr>(BIGINT(),
variant(boundType.preceding().offset())));
+ if (boundType.preceding().has_offset()) {
+ return std::make_tuple(
+ core::WindowNode::BoundType::kPreceding,
+ std::make_shared<core::ConstantTypedExpr>(BIGINT(),
variant(boundType.preceding().offset())));
+ } else {
+ return std::make_tuple(
+ core::WindowNode::BoundType::kPreceding,
+ exprConverter_->toVeloxExpr(boundType.preceding().ref(),
inputType));
+ }
Review Comment:
done.
I tried to reuse the logic here, but since the type of
`boundType.preceding()` and `boundType.following()` are different, so I have to
pass the `offset` and `ref` instead of the expression itself
##########
gluten-core/src/main/resources/substrait/proto/substrait/algebra.proto:
##########
@@ -989,18 +989,28 @@ message Expression {
message Bound {
// Defines that the bound extends this far back from the current record.
message Preceding {
- // A strictly positive integer specifying the number of records that
- // the window extends back from the current record. Required. Use
- // CurrentRow for offset zero and Following for negative offsets.
- int64 offset = 1;
+ oneof kind {
+ // A strictly positive integer specifying the number of records that
+ // the window extends back from the current record. Use
+ // CurrentRow for offset zero and Following for negative offsets.
+ int64 offset = 1;
+
+ // the reference to pre-project range frame boundary.
+ Expression ref = 2;
+ }
Review Comment:
added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
