cloud-fan commented on code in PR #52153:
URL: https://github.com/apache/spark/pull/52153#discussion_r2312822936


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala:
##########
@@ -1925,6 +1934,21 @@ object RepartitionByExpression {
       numPartitions: Int): RepartitionByExpression = {
     RepartitionByExpression(partitionExpressions, child, Some(numPartitions))
   }
+
+  // 4-parameter apply method for backwards compatibility with existing case 
matches
+  def apply(
+      partitionExpressions: Seq[Expression],
+      child: LogicalPlan,
+      optNumPartitions: Option[Int],
+      optAdvisoryPartitionSize: Option[Long]): RepartitionByExpression = {
+    RepartitionByExpression(partitionExpressions, child, optNumPartitions,
+      optAdvisoryPartitionSize, directPassthrough = false)
+  }
+
+  def unapply(r: RepartitionByExpression)

Review Comment:
   OK now I'm reconsidering it. By adding a new flag in 
`RepartitionByExpression`, we need to update all its pattern matches, which is 
4 or 5 places in Spark, and probably more in the third party Spark plugins. We 
also need to add new type check for the id pass-through expression in 
`CheckAnalysis`.
   
   We can avoid these troubles by adding a new expression to hold the id 
pass-through expression, which is more self-contained.
   
   cc @HyukjinKwon 



-- 
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]

Reply via email to