zhztheplayer commented on code in PR #11455:
URL:
https://github.com/apache/incubator-gluten/pull/11455#discussion_r2713844340
##########
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/enumerated/planner/metadata/GlutenMetadataModel.scala:
##########
@@ -33,10 +34,12 @@ object GlutenMetadataModel extends Logging {
case other =>
GlutenMetadata(
Schema(other.output),
-
other.logicalLink.map(LogicalLink(_)).getOrElse(LogicalLink.notFound))
+
other.logicalLink.map(LogicalLink(_)).getOrElse(LogicalLink.notFound),
+ other.outputPartitioning)
Review Comment:
This change may not break anything, but partitioning is not usually
considered a logical property in terms of a Cascades-style query optimizer like
RAS, so it's essentially not a correct change.
BTW, I was considering completely removing RAS as 1. no active users 2. no
one else in the community could maintain the code forward with enough
search-based optimizer framework knowledge. More contexts should reveal
afterwards.
##########
gluten-substrait/src/main/scala/org/apache/gluten/execution/RangeExecBaseTransformer.scala:
##########
@@ -33,7 +34,9 @@ abstract class ColumnarRangeBaseExec(
numSlices: Int,
numElements: BigInt,
outputAttributes: Seq[Attribute],
- child: Seq[SparkPlan])
+ child: Seq[SparkPlan],
+ override val outputOrdering: Seq[SortOrder],
+ override val outputPartitioning: Partitioning)
Review Comment:
Why range has children? Can we remove?
--
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]
