You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/08/06 18:28:41 UTC

[GitHub] [spark] maryannxue opened a new pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

maryannxue opened a new pull request #33671:
URL: https://github.com/apache/spark/pull/33671


   ### What changes were proposed in this pull request?
   This PR fixes an existing correctness issue where a non-deterministic With-CTE can be executed multiple times producing different results, by deferring the inline of With-CTE to after the analysis stage. This fix also provides the future opportunity of performance improvement by executing deterministic With-CTEs only once in some circumstances.
   
   The major changes include:
   1. Added new With-CTE logical nodes: `CTERelationDef`, `CTERelationRef`, `WithCTE`. Each `CTERelationDef` has a unique ID and the mapping between CTE def and CTE ref is based on IDs rather than names. `WithCTE` is a resolved version of `With`, only that: 1) `WithCTE` is a multi-children logical node so that most logical rules can automatically apply to CTE defs; 2) In the main query and each subquery, there can only be at most one `WithCTE`, which means nested With-CTEs are combined.
   2. Changed `CTESubstitution` rule so that if NOT in legacy mode, CTE defs will not be inlined immediately, but rather transformed into a `CTERelationRef` per reference.
   3. Added new With-CTE rules: 1) `ResolveWithCTE` - to update `CTERelationRef`s with resolved output from corresponding `CTERelationDef`s; 2) `InlineCTE` - to inline deterministic CTEs or non-deterministic CTEs with only ONE reference; 3) `UpdateCTERelationStats` - to update stats for `CTERelationRef`s that are not inlined.
   4. Added a CTE physical planning strategy to plan `CTERelationRef`s as an independent shuffle with round-robin partitioning so that such CTEs will only be materialized once and different references will later be a shuffle reuse.
   
   A current limitation is that With-CTEs mixed with SQL commands or DMLs will still go through the old inline code path because of our non-standard language specs and not-unified command/DML interfaces.
   
   ### Why are the changes needed?
   This is a correctness issue. Non-deterministic CTEs should produce the same output regardless of how many times it is referenced/used in query, while under the current implementation there is no such guarantee and would lead to incorrect query results.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   Added UTs.
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-898009670


   **[Test build #142393 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142393/testReport)** for PR 33671 at commit [`4cc52f7`](https://github.com/apache/spark/commit/4cc52f74ab8a12599b74ae163e8f4065a99e359d).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maryannxue commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
maryannxue commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r686939363



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -20,25 +20,60 @@ package org.apache.spark.sql.catalyst.analysis
 import scala.collection.mutable
 
 import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
-import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias, With}
+import org.apache.spark.sql.catalyst.plans.logical.{Command, CTERelationDef, CTERelationRef, InsertIntoDir, LogicalPlan, ParsedStatement, SubqueryAlias, UnresolvedWith, WithCTE}
 import org.apache.spark.sql.catalyst.rules.Rule
 import org.apache.spark.sql.catalyst.trees.TreePattern._
 import org.apache.spark.sql.errors.QueryCompilationErrors
 import org.apache.spark.sql.internal.SQLConf.{LEGACY_CTE_PRECEDENCE_POLICY, LegacyBehaviorPolicy}
 
 /**
- * Analyze WITH nodes and substitute child plan with CTE definitions.
+ * Analyze WITH nodes and substitute child plan with CTE references or CTE definitions depending
+ * on the conditions below:
+ * 1. If in legacy mode, or if the query is a SQL command or DML statement, replace with CTE
+ *    definitions, i.e., inline CTEs.
+ * 2. Otherwise, replace with CTE references `CTERelationRef`s. The decision to inline or not
+ *    inline will be made later by the rule `InlineCTE` after query analysis.
+ *
+ * All the CTE definitions that are not inlined after this substitution will be grouped together
+ * under one `WithCTE` node for each of the main query and the subqueries. Any of the main query
+ * or the subqueries that do not contain CTEs or have had all CTEs inlined will obviously not have
+ * any `WithCTE` nodes. If any though, the `WithCTE` node will be in the same place as where the
+ * outermost `With` node once was.
+ *
+ * The CTE definitions in a `WithCTE` node are kept in the order of how they have been resolved.
+ * That means the CTE definitions are guaranteed to be in topological order base on their
+ * dependency for any valid CTE query (i.e., given CTE definitions A and B with B referencing A,
+ * A is guaranteed to appear before B). Otherwise, it must be an invalid user query, and an
+ * analysis exception will be thrown later by relation resolving rules.
  */
 object CTESubstitution extends Rule[LogicalPlan] {
   def apply(plan: LogicalPlan): LogicalPlan = {
-    LegacyBehaviorPolicy.withName(conf.getConf(LEGACY_CTE_PRECEDENCE_POLICY)) match {
-      case LegacyBehaviorPolicy.EXCEPTION =>
-        assertNoNameConflictsInCTE(plan)
-        traverseAndSubstituteCTE(plan)
-      case LegacyBehaviorPolicy.LEGACY =>
-        legacyTraverseAndSubstituteCTE(plan)
-      case LegacyBehaviorPolicy.CORRECTED =>
-        traverseAndSubstituteCTE(plan)
+    val isCommand = plan.find {

Review comment:
       Maybe I'm missing sth, but I don't see how so.




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897730960


   **[Test build #142386 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142386/testReport)** for PR 33671 at commit [`3746d76`](https://github.com/apache/spark/commit/3746d76528257c2651a6e3834e7f764921c6bb17).
    * This patch **fails to build**.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894489103


   **[Test build #142176 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142176/testReport)** for PR 33671 at commit [`7ab352a`](https://github.com/apache/spark/commit/7ab352adf974b25259cf9c5cf8112017d09b0d76).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896114206


   **[Test build #142277 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142277/testReport)** for PR 33671 at commit [`2c30d4a`](https://github.com/apache/spark/commit/2c30d4acd89308a6b3e190bbd7cf95d40097565f).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-895315524


   **[Test build #142229 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142229/testReport)** for PR 33671 at commit [`dd4fc28`](https://github.com/apache/spark/commit/dd4fc28c3d2ee2742195cffccd6f09f2a7203db5).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] peter-toth commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
peter-toth commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r684794342



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -20,25 +20,60 @@ package org.apache.spark.sql.catalyst.analysis
 import scala.collection.mutable
 
 import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
-import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias, With}
+import org.apache.spark.sql.catalyst.plans.logical.{Command, CTERelationDef, CTERelationRef, InsertIntoDir, LogicalPlan, ParsedStatement, SubqueryAlias, UnresolvedWith, WithCTE}
 import org.apache.spark.sql.catalyst.rules.Rule
 import org.apache.spark.sql.catalyst.trees.TreePattern._
 import org.apache.spark.sql.errors.QueryCompilationErrors
 import org.apache.spark.sql.internal.SQLConf.{LEGACY_CTE_PRECEDENCE_POLICY, LegacyBehaviorPolicy}
 
 /**
- * Analyze WITH nodes and substitute child plan with CTE definitions.
+ * Analyze WITH nodes and substitute child plan with CTE references or CTE definitions depending
+ * on the conditions below:
+ * 1. If in legacy mode, or if the query is a SQL command or DML statement, replace with CTE
+ *    definitions, i.e., inline CTEs.
+ * 2. Otherwise, replace with CTE references `CTERelationRef`s. The decision to inline or not
+ *    inline will be made later by the rule `InlineCTE` after query analysis.
+ *
+ * All the CTE definitions that are not inlined after this substitution will be grouped together
+ * under one `WithCTE` node for each of the main query and the subqueries. Any of the main query
+ * or the subqueries that do not contain CTEs or have had all CTEs inlined will obviously not have
+ * any `WithCTE` nodes. If any though, the `WithCTE` node will be in the same place as where the
+ * outermost `With` node once was.
+ *
+ * The CTE definitions in a `WithCTE` node are kept in the order of how they have been resolved.
+ * That means the CTE definitions are guaranteed to be in topological order base on their
+ * dependency for any valid CTE query (i.e., given CTE definitions A and B with B referencing A,
+ * A is guaranteed to appear before B). Otherwise, it must be an invalid user query, and an
+ * analysis exception will be thrown later by relation resolving rules.
  */
 object CTESubstitution extends Rule[LogicalPlan] {
   def apply(plan: LogicalPlan): LogicalPlan = {
-    LegacyBehaviorPolicy.withName(conf.getConf(LEGACY_CTE_PRECEDENCE_POLICY)) match {
-      case LegacyBehaviorPolicy.EXCEPTION =>
-        assertNoNameConflictsInCTE(plan)
-        traverseAndSubstituteCTE(plan)
-      case LegacyBehaviorPolicy.LEGACY =>
-        legacyTraverseAndSubstituteCTE(plan)
-      case LegacyBehaviorPolicy.CORRECTED =>
-        traverseAndSubstituteCTE(plan)
+    val isCommand = plan.find {
+      case _: Command | _: ParsedStatement | _: InsertIntoDir => true
+      case _ => false
+    }.isDefined
+    val cteDefs = mutable.ArrayBuffer.empty[CTERelationDef]
+    val (substituted, lastSubstituted) =
+      LegacyBehaviorPolicy.withName(conf.getConf(LEGACY_CTE_PRECEDENCE_POLICY)) match {
+        case LegacyBehaviorPolicy.EXCEPTION =>
+          assertNoNameConflictsInCTE(plan)
+          traverseAndSubstituteCTE(plan, isCommand, cteDefs)
+        case LegacyBehaviorPolicy.LEGACY =>
+          (legacyTraverseAndSubstituteCTE(plan, cteDefs), None)
+        case LegacyBehaviorPolicy.CORRECTED =>
+          traverseAndSubstituteCTE(plan, isCommand, cteDefs)
+    }
+    if (cteDefs.isEmpty) {
+      substituted
+    } else if (substituted eq lastSubstituted.get) {
+      WithCTE(substituted, cteDefs)
+    } else {
+      var done = false
+      substituted.resolveOperators {

Review comment:
       Maybe we could use `substituted.resolveOperatorsWithPruning(_ => !done) {` to break out?




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687082625



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveWithCTE.scala
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.analysis
+
+import scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
+import org.apache.spark.sql.catalyst.plans.logical.{CTERelationDef, CTERelationRef, LogicalPlan, WithCTE}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.TreePattern.{CTE, PLAN_EXPRESSION}
+
+/**
+ * Updates CTE references with the resolve output attributes of corresponding CTE definitions.
+ */
+object ResolveWithCTE extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    if (plan.containsAllPatterns(CTE)) {
+      val cteDefMap = mutable.HashMap.empty[Long, CTERelationDef]
+      resolveWithCTE(plan, cteDefMap)
+    } else {
+      plan
+    }
+  }
+
+  private def resolveWithCTE(
+      plan: LogicalPlan,
+      cteDefMap: mutable.HashMap[Long, CTERelationDef]): LogicalPlan = {
+    plan.resolveOperatorsDownWithPruning(_.containsAllPatterns(CTE)) {
+      case w @ WithCTE(_, cteDefs) =>
+        cteDefs.foreach { cteDef =>
+          if (cteDef.resolved) {
+            cteDefMap.put(cteDef.id, cteDef)
+          }
+        }
+        w
+
+      case ref: CTERelationRef if !ref.resolved =>
+        cteDefMap.get(ref.cteId).map { cteDef =>
+          CTERelationRef(cteDef.id, cteDef.resolved, cteDef.output)
+        }.getOrElse {
+          ref
+        }
+
+      case other =>
+        other.transformExpressionsWithPruning(_.containsAllPatterns(PLAN_EXPRESSION, CTE)) {
+          case e: SubqueryExpression => e.withNewPlan(resolveWithCTE(e.plan, cteDefMap))

Review comment:
       oh I see, makes sense




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897164050


   **[Test build #142349 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142349/testReport)** for PR 33671 at commit [`09c1c27`](https://github.com/apache/spark/commit/09c1c274af7e3883cba326a50d3e24109f2c456b).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896162376


   **[Test build #142283 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142283/testReport)** for PR 33671 at commit [`92262a2`](https://github.com/apache/spark/commit/92262a2877dbc3c9275fc610366e851e11f933c1).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687462691



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/subquery.scala
##########
@@ -68,7 +68,7 @@ abstract class PlanExpression[T <: QueryPlan[_]] extends Expression {
  */
 abstract class SubqueryExpression(
     plan: LogicalPlan,
-    outerAttrs: Seq[Expression],
+    val outerAttrs: Seq[Expression],

Review comment:
       nit: this needs to touch all the subclasses, which is a bit messy. How about we just add a new method in this class?
   ```
   def isCorrelated: Boolean = outerAttrs.nonEmpty
   ```




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897815198


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46894/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896193859


   **[Test build #142277 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142277/testReport)** for PR 33671 at commit [`2c30d4a`](https://github.com/apache/spark/commit/2c30d4acd89308a6b3e190bbd7cf95d40097565f).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894539019


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46688/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894460290


   **[Test build #142172 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142172/testReport)** for PR 33671 at commit [`4f24104`](https://github.com/apache/spark/commit/4f24104e2af4ed07dbaa004e5e15ebc1e92cf2de).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896344917


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142283/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897801402


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46894/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894507034


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46687/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897164050


   **[Test build #142349 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142349/testReport)** for PR 33671 at commit [`09c1c27`](https://github.com/apache/spark/commit/09c1c274af7e3883cba326a50d3e24109f2c456b).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896230908


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46790/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894514717


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46687/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894463690


   **[Test build #142175 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142175/testReport)** for PR 33671 at commit [`4d08307`](https://github.com/apache/spark/commit/4d08307113e8327c32e394b94164ee51ddb58d22).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897725942


   **[Test build #142386 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142386/testReport)** for PR 33671 at commit [`3746d76`](https://github.com/apache/spark/commit/3746d76528257c2651a6e3834e7f764921c6bb17).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896343274


   **[Test build #142283 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142283/testReport)** for PR 33671 at commit [`92262a2`](https://github.com/apache/spark/commit/92262a2877dbc3c9275fc610366e851e11f933c1).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896150301


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46785/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687031657



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
##########
@@ -4215,6 +4215,255 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-36447: non-deterministic CTE dedup") {

Review comment:
       Shall we create a new `CTEInlineSuite`?




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894460290


   **[Test build #142172 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142172/testReport)** for PR 33671 at commit [`4f24104`](https://github.com/apache/spark/commit/4f24104e2af4ed07dbaa004e5e15ebc1e92cf2de).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zinking commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
zinking commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-995756427


   @maryannxue  I find after this change, the eventually expanded query now contains duplicated expression Ids. 
   
   is this expected? I am thinking of the assumption expression id would be unique globally.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687020021



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveWithCTE.scala
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.analysis
+
+import scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
+import org.apache.spark.sql.catalyst.plans.logical.{CTERelationDef, CTERelationRef, LogicalPlan, WithCTE}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.TreePattern.{CTE, PLAN_EXPRESSION}
+
+/**
+ * Updates CTE references with the resolve output attributes of corresponding CTE definitions.
+ */
+object ResolveWithCTE extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    if (plan.containsAllPatterns(CTE)) {
+      val cteDefMap = mutable.HashMap.empty[Long, CTERelationDef]
+      resolveWithCTE(plan, cteDefMap)
+    } else {
+      plan
+    }
+  }
+
+  private def resolveWithCTE(
+      plan: LogicalPlan,
+      cteDefMap: mutable.HashMap[Long, CTERelationDef]): LogicalPlan = {
+    plan.resolveOperatorsDownWithPruning(_.containsAllPatterns(CTE)) {
+      case w @ WithCTE(_, cteDefs) =>
+        cteDefs.foreach { cteDef =>
+          if (cteDef.resolved) {
+            cteDefMap.put(cteDef.id, cteDef)
+          }
+        }
+        w
+
+      case ref: CTERelationRef if !ref.resolved =>
+        cteDefMap.get(ref.cteId).map { cteDef =>
+          CTERelationRef(cteDef.id, cteDef.resolved, cteDef.output)
+        }.getOrElse {
+          ref
+        }
+
+      case other =>
+        other.transformExpressionsWithPruning(_.containsAllPatterns(PLAN_EXPRESSION, CTE)) {
+          case e: SubqueryExpression => e.withNewPlan(resolveWithCTE(e.plan, cteDefMap))

Review comment:
       If the main query has more than one subqueries, when resolving the second subquery, the `cteDefMap` will contain CTE defs from the first subquery. I think we should clone the map here?




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maryannxue commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
maryannxue commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687081316



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveWithCTE.scala
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.analysis
+
+import scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
+import org.apache.spark.sql.catalyst.plans.logical.{CTERelationDef, CTERelationRef, LogicalPlan, WithCTE}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.TreePattern.{CTE, PLAN_EXPRESSION}
+
+/**
+ * Updates CTE references with the resolve output attributes of corresponding CTE definitions.
+ */
+object ResolveWithCTE extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    if (plan.containsAllPatterns(CTE)) {
+      val cteDefMap = mutable.HashMap.empty[Long, CTERelationDef]
+      resolveWithCTE(plan, cteDefMap)
+    } else {
+      plan
+    }
+  }
+
+  private def resolveWithCTE(
+      plan: LogicalPlan,
+      cteDefMap: mutable.HashMap[Long, CTERelationDef]): LogicalPlan = {
+    plan.resolveOperatorsDownWithPruning(_.containsAllPatterns(CTE)) {
+      case w @ WithCTE(_, cteDefs) =>
+        cteDefs.foreach { cteDef =>
+          if (cteDef.resolved) {
+            cteDefMap.put(cteDef.id, cteDef)
+          }
+        }
+        w
+
+      case ref: CTERelationRef if !ref.resolved =>
+        cteDefMap.get(ref.cteId).map { cteDef =>
+          CTERelationRef(cteDef.id, cteDef.resolved, cteDef.output)
+        }.getOrElse {
+          ref
+        }
+
+      case other =>
+        other.transformExpressionsWithPruning(_.containsAllPatterns(PLAN_EXPRESSION, CTE)) {
+          case e: SubqueryExpression => e.withNewPlan(resolveWithCTE(e.plan, cteDefMap))

Review comment:
       It's not necessary, right? The real resolving has happened in CTESubstitution earlier. Now there's a strict 1 to 1 ID mapping, so the map can only contain some unrelated CTE defs at its worst.




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896118055


   **[Test build #142278 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142278/testReport)** for PR 33671 at commit [`7443fb9`](https://github.com/apache/spark/commit/7443fb995066aab1113f7bf78fe9e3dcbb41de3b).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] peter-toth commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
peter-toth commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r690408962



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -130,23 +169,36 @@ object CTESubstitution extends Rule[LogicalPlan] {
    * @param plan the plan to be traversed
    * @return the plan where CTE substitution is applied

Review comment:
       As the return value of `traverseAndSubstituteCTE` changed a bit, this docs could use some update.




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-895360676


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46742/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894465594


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46684/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897866655


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46899/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897831294


   **[Test build #142393 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142393/testReport)** for PR 33671 at commit [`4cc52f7`](https://github.com/apache/spark/commit/4cc52f74ab8a12599b74ae163e8f4065a99e359d).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896298116






-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897292315


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142349/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897220042


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46858/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] peter-toth commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
peter-toth commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-900332904


   Late LGTM. Just a very minor comment.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894563307


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142176/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896162376


   **[Test build #142283 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142283/testReport)** for PR 33671 at commit [`92262a2`](https://github.com/apache/spark/commit/92262a2877dbc3c9275fc610366e851e11f933c1).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-898010498


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142393/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan closed pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #33671:
URL: https://github.com/apache/spark/pull/33671


   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jaceklaskowski commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
jaceklaskowski commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r686900021



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -20,25 +20,60 @@ package org.apache.spark.sql.catalyst.analysis
 import scala.collection.mutable
 
 import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
-import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias, With}
+import org.apache.spark.sql.catalyst.plans.logical.{Command, CTERelationDef, CTERelationRef, InsertIntoDir, LogicalPlan, ParsedStatement, SubqueryAlias, UnresolvedWith, WithCTE}
 import org.apache.spark.sql.catalyst.rules.Rule
 import org.apache.spark.sql.catalyst.trees.TreePattern._
 import org.apache.spark.sql.errors.QueryCompilationErrors
 import org.apache.spark.sql.internal.SQLConf.{LEGACY_CTE_PRECEDENCE_POLICY, LegacyBehaviorPolicy}
 
 /**
- * Analyze WITH nodes and substitute child plan with CTE definitions.
+ * Analyze WITH nodes and substitute child plan with CTE references or CTE definitions depending
+ * on the conditions below:
+ * 1. If in legacy mode, or if the query is a SQL command or DML statement, replace with CTE
+ *    definitions, i.e., inline CTEs.
+ * 2. Otherwise, replace with CTE references `CTERelationRef`s. The decision to inline or not
+ *    inline will be made later by the rule `InlineCTE` after query analysis.
+ *
+ * All the CTE definitions that are not inlined after this substitution will be grouped together
+ * under one `WithCTE` node for each of the main query and the subqueries. Any of the main query
+ * or the subqueries that do not contain CTEs or have had all CTEs inlined will obviously not have
+ * any `WithCTE` nodes. If any though, the `WithCTE` node will be in the same place as where the
+ * outermost `With` node once was.
+ *
+ * The CTE definitions in a `WithCTE` node are kept in the order of how they have been resolved.
+ * That means the CTE definitions are guaranteed to be in topological order base on their
+ * dependency for any valid CTE query (i.e., given CTE definitions A and B with B referencing A,
+ * A is guaranteed to appear before B). Otherwise, it must be an invalid user query, and an
+ * analysis exception will be thrown later by relation resolving rules.
  */
 object CTESubstitution extends Rule[LogicalPlan] {
   def apply(plan: LogicalPlan): LogicalPlan = {
-    LegacyBehaviorPolicy.withName(conf.getConf(LEGACY_CTE_PRECEDENCE_POLICY)) match {
-      case LegacyBehaviorPolicy.EXCEPTION =>
-        assertNoNameConflictsInCTE(plan)
-        traverseAndSubstituteCTE(plan)
-      case LegacyBehaviorPolicy.LEGACY =>
-        legacyTraverseAndSubstituteCTE(plan)
-      case LegacyBehaviorPolicy.CORRECTED =>
-        traverseAndSubstituteCTE(plan)
+    val isCommand = plan.find {

Review comment:
       `plan.collectFirst` would be quicker and less memory-hungry, wouldn't it?




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-895399854


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46742/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896114206


   **[Test build #142277 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142277/testReport)** for PR 33671 at commit [`2c30d4a`](https://github.com/apache/spark/commit/2c30d4acd89308a6b3e190bbd7cf95d40097565f).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897731041


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142386/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894464655


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142172/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687026727



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -625,7 +627,78 @@ case class With(child: LogicalPlan, cteRelations: Seq[(String, SubqueryAlias)])
 
   override def innerChildren: Seq[LogicalPlan] = cteRelations.map(_._2)
 
-  override protected def withNewChildInternal(newChild: LogicalPlan): With = copy(child = newChild)
+  override protected def withNewChildInternal(newChild: LogicalPlan): UnresolvedWith =
+    copy(child = newChild)
+}
+
+/**
+ * A wrapper for CTE definition plan with a unique ID.
+ * @param child The CTE definition query plan.
+ * @param id    The unique ID for this CTE definition.
+ */
+case class CTERelationDef(child: LogicalPlan, id: Long = CTERelationDef.newId) extends UnaryNode {
+
+  final override val nodePatterns: Seq[TreePattern] = Seq(CTE)
+
+  override protected def withNewChildInternal(newChild: LogicalPlan): LogicalPlan =
+    copy(child = newChild)
+
+  override def output: Seq[Attribute] = if (resolved) child.output else Nil
+}
+
+object CTERelationDef {
+  private val curId = new java.util.concurrent.atomic.AtomicLong()
+  def newId: Long = curId.getAndIncrement()
+}
+
+/**
+ * Represents the relation of a CTE reference.
+ * @param cteId     The ID of the corresponding CTE definition.
+ * @param _resolved Whether this reference is resolved.
+ * @param output    The output attributes of this CTE reference, which can be different from
+ *                  the output of its corresponding CTE definition after attribute de-duplication.
+ * @param statsOpt  The optional statistics inferred from the corresponding CTE definition.
+ */
+case class CTERelationRef(
+  cteId: Long,

Review comment:
       nit: 4 spaces indentation




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894464655


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142172/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-898168189


   thanks, merging to master/3.2 (since it's a correctness fix)! 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] hvanhovell commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
hvanhovell commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687098041



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -662,6 +663,36 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
     }
   }
 
+  /**
+   * Strategy to plan CTE relations left not inlined.
+   */
+  object WithCTEStrategy extends Strategy {
+    override def apply(plan: LogicalPlan): Seq[SparkPlan] = plan match {
+      case WithCTE(plan, cteDefs) =>
+        val cteMap = QueryExecution.cteMap
+        cteDefs.foreach { cteDef =>
+          cteMap.put(cteDef.id, cteDef)
+        }
+        planLater(plan) :: Nil
+
+      case r: CTERelationRef =>
+        val ctePlan = QueryExecution.cteMap(r.cteId).child
+        val projectList = r.output.zip(ctePlan.output).map { case (tgtAttr, srcAttr) =>
+          Alias(srcAttr, tgtAttr.name)(exprId = tgtAttr.exprId)
+        }
+        val newPlan = Project(projectList, ctePlan)
+        // Plan CTE ref as a repartition shuffle so that all refs of the same CTE def will share
+        // an Exchange reuse at runtime.
+        // TODO create a new identity partitioning instead of using RoundRobinPartitioning.
+        exchange.ShuffleExchangeExec(
+          RoundRobinPartitioning(conf.numShufflePartitions),

Review comment:
       RoundRobin sorts data before shuffling right? That will slow things a lot.




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894465579


   Kubernetes integration test unable to build dist.
   
   exiting with code: 1
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46684/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896140228


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46786/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896298116


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142278/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maryannxue commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
maryannxue commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687151502



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -662,6 +663,36 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
     }
   }
 
+  /**
+   * Strategy to plan CTE relations left not inlined.
+   */
+  object WithCTEStrategy extends Strategy {
+    override def apply(plan: LogicalPlan): Seq[SparkPlan] = plan match {
+      case WithCTE(plan, cteDefs) =>
+        val cteMap = QueryExecution.cteMap
+        cteDefs.foreach { cteDef =>
+          cteMap.put(cteDef.id, cteDef)
+        }
+        planLater(plan) :: Nil
+
+      case r: CTERelationRef =>
+        val ctePlan = QueryExecution.cteMap(r.cteId).child
+        val projectList = r.output.zip(ctePlan.output).map { case (tgtAttr, srcAttr) =>
+          Alias(srcAttr, tgtAttr.name)(exprId = tgtAttr.exprId)
+        }
+        val newPlan = Project(projectList, ctePlan)
+        // Plan CTE ref as a repartition shuffle so that all refs of the same CTE def will share
+        // an Exchange reuse at runtime.
+        // TODO create a new identity partitioning instead of using RoundRobinPartitioning.
+        exchange.ShuffleExchangeExec(
+          RoundRobinPartitioning(conf.numShufflePartitions),

Review comment:
       Yes, so that's why we put a TODO there.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -662,6 +663,36 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
     }
   }
 
+  /**
+   * Strategy to plan CTE relations left not inlined.
+   */
+  object WithCTEStrategy extends Strategy {
+    override def apply(plan: LogicalPlan): Seq[SparkPlan] = plan match {
+      case WithCTE(plan, cteDefs) =>
+        val cteMap = QueryExecution.cteMap
+        cteDefs.foreach { cteDef =>
+          cteMap.put(cteDef.id, cteDef)
+        }
+        planLater(plan) :: Nil
+
+      case r: CTERelationRef =>
+        val ctePlan = QueryExecution.cteMap(r.cteId).child
+        val projectList = r.output.zip(ctePlan.output).map { case (tgtAttr, srcAttr) =>
+          Alias(srcAttr, tgtAttr.name)(exprId = tgtAttr.exprId)
+        }
+        val newPlan = Project(projectList, ctePlan)
+        // Plan CTE ref as a repartition shuffle so that all refs of the same CTE def will share
+        // an Exchange reuse at runtime.
+        // TODO create a new identity partitioning instead of using RoundRobinPartitioning.
+        exchange.ShuffleExchangeExec(
+          RoundRobinPartitioning(conf.numShufflePartitions),

Review comment:
       Yes, that's why we put a TODO there.




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897288003


   **[Test build #142349 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142349/testReport)** for PR 33671 at commit [`09c1c27`](https://github.com/apache/spark/commit/09c1c274af7e3883cba326a50d3e24109f2c456b).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896243528


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46790/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-895399966






-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897292315


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142349/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894513564


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46688/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894514717


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46687/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897815198


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46894/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maryannxue commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
maryannxue commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687077048



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
##########
@@ -4215,6 +4215,255 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-36447: non-deterministic CTE dedup") {

Review comment:
       Good idea




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896297072


   **[Test build #142278 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142278/testReport)** for PR 33671 at commit [`7443fb9`](https://github.com/apache/spark/commit/7443fb995066aab1113f7bf78fe9e3dcbb41de3b).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687025603



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InlineCTE.scala
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.optimizer
+
+import scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.analysis.DeduplicateRelations
+import org.apache.spark.sql.catalyst.expressions.{Alias, OuterReference, SubqueryExpression}
+import org.apache.spark.sql.catalyst.plans.Inner
+import org.apache.spark.sql.catalyst.plans.logical.{CTERelationDef, CTERelationRef, Join, JoinHint, LogicalPlan, Project, Subquery, WithCTE}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.TreePattern.{CTE, PLAN_EXPRESSION}
+
+/**
+ * Inlines CTE definitions into corresponding references if either of the conditions satisfies:
+ * 1. The CTE definition does not contain any non-deterministic expressions. If this CTE
+ *    definition references another CTE definition that has non-deterministic expressions, it
+ *    is still OK to inline the current CTE definition.
+ * 2. The CTE definition is only referenced once throughout the main query and all the subqueries.
+ *
+ * In addition, due to the complexity of correlated subqueries, all CTE references in correlated
+ * subqueries are inlined regardless of the conditions above.
+ */
+object InlineCTE extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    if (!plan.isInstanceOf[Subquery] && plan.containsPattern(CTE)) {
+      val cteMap = mutable.HashMap.empty[Long, (CTERelationDef, Int)]
+      buildCTEMap(plan, cteMap)
+      inlineCTE(plan, cteMap, forceInline = false)
+    } else {
+      plan
+    }
+  }
+
+  private def shouldInline(cteDef: CTERelationDef, refCount: Int): Boolean = {
+    // We do not need to check enclosed `CTERelationRef`s for `deterministic` or `OuterReference`,
+    // because:
+    // 1) It is fine to inline a CTE if it references another CTE that is non-deterministic;
+    // 2) Any `CTERelationRef` that contains `OuterReference` would have been inlined first.
+    refCount == 1 ||
+      cteDef.child.find(_.expressions.exists(!_.deterministic)).isEmpty ||
+      cteDef.child.find(_.expressions.exists(_.isInstanceOf[OuterReference])).isDefined
+  }
+
+  private def buildCTEMap(
+      plan: LogicalPlan,
+      cteMap: mutable.HashMap[Long, (CTERelationDef, Int)]): Unit = {
+    plan match {
+      case WithCTE(_, cteDefs) =>
+        cteDefs.foreach { cteDef =>
+          cteMap.put(cteDef.id, (cteDef, 0))
+        }
+
+      case ref: CTERelationRef =>
+        val (cteDef, refCount) = cteMap(ref.cteId)
+        cteMap.update(ref.cteId, (cteDef, refCount + 1))
+
+      case _ =>
+    }
+
+    if (plan.containsPattern(CTE)) {
+      plan.children.foreach { child =>
+        buildCTEMap(child, cteMap)
+      }
+
+      plan.expressions.foreach { expr =>
+        if (expr.containsAllPatterns(PLAN_EXPRESSION, CTE)) {
+          expr.foreach {
+            case e: SubqueryExpression =>
+              buildCTEMap(e.plan, cteMap)
+            case _ =>
+          }
+        }
+      }
+    }
+  }
+
+  private def inlineCTE(
+      plan: LogicalPlan,
+      cteMap: mutable.HashMap[Long, (CTERelationDef, Int)],
+      forceInline: Boolean): LogicalPlan = {
+    val (stripped, notInlined) = plan match {
+      case WithCTE(child, cteDefs) =>
+        val notInlined = mutable.ArrayBuffer.empty[CTERelationDef]
+        cteDefs.foreach { cteDef =>
+          val (cte, refCount) = cteMap(cteDef.id)
+          if (refCount > 0) {
+            val inlined = cte.copy(child = inlineCTE(cte.child, cteMap, forceInline))
+            cteMap.update(cteDef.id, (inlined, refCount))
+            if (!forceInline && !shouldInline(inlined, refCount)) {
+              notInlined.append(inlined)
+            }
+          }
+        }
+        (inlineCTE(child, cteMap, forceInline), notInlined.toSeq)
+
+      case ref: CTERelationRef =>
+        val (cteDef, refCount) = cteMap(ref.cteId)
+        val newRef = if (forceInline || shouldInline(cteDef, refCount)) {
+          if (ref.outputSet == cteDef.outputSet) {
+            cteDef.child
+          } else {
+            val ctePlan = DeduplicateRelations(
+              Join(cteDef.child, cteDef.child, Inner, None, JoinHint(None, None))).children(1)
+            val projectList = ref.output.zip(ctePlan.output).map { case (tgtAttr, srcAttr) =>
+              Alias(srcAttr, tgtAttr.name)(exprId = tgtAttr.exprId)
+            }
+            Project(projectList, ctePlan)
+          }
+        } else {
+          ref
+        }
+        (newRef, Seq.empty)
+
+      case _ if plan.containsPattern(CTE) =>
+        val newPlan = plan
+          .withNewChildren(plan.children.map(child => inlineCTE(child, cteMap, forceInline)))
+          .transformExpressionsWithPruning(_.containsAllPatterns(PLAN_EXPRESSION, CTE)) {
+            case e: SubqueryExpression =>
+              val forceInline =
+                e.plan.find(_.expressions.exists(_.isInstanceOf[OuterReference])).nonEmpty

Review comment:
       There is an easier way to check correlated subquery: `e.outerAttrs.nonEmpty`




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894549412


   **[Test build #142175 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142175/testReport)** for PR 33671 at commit [`4d08307`](https://github.com/apache/spark/commit/4d08307113e8327c32e394b94164ee51ddb58d22).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897899666


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46899/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-895399966


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46742/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896201725


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46790/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894529255


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46688/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894539019


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46688/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894563307


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142176/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maryannxue commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
maryannxue commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687078471



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala
##########
@@ -623,11 +625,6 @@ case class AdaptiveSparkPlanExec(
         val newLogicalPlan = logicalPlan.transformDown {
           case p if p.eq(logicalNode) => newLogicalNode
         }
-        assert(newLogicalPlan != logicalPlan,

Review comment:
       Because stages in CTE defs with multiple references can be "replaced" more than once, but those are just the reuses of the same exchange, so we can ignore them.




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] peter-toth commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
peter-toth commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894818221


   Thanks @maryannxue for pinging me. Unfortunately, I can take a closer look at this PR only the week after next...


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896118055


   **[Test build #142278 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142278/testReport)** for PR 33671 at commit [`7443fb9`](https://github.com/apache/spark/commit/7443fb995066aab1113f7bf78fe9e3dcbb41de3b).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maryannxue commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
maryannxue commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-895315199


   Never mind, @peter-toth! Hopefully the added logical nodes make sense to you. We can always improve the implementation later on.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897195253


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46858/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896203505


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142277/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-898010498


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142393/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687030254



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala
##########
@@ -623,11 +625,6 @@ case class AdaptiveSparkPlanExec(
         val newLogicalPlan = logicalPlan.transformDown {
           case p if p.eq(logicalNode) => newLogicalNode
         }
-        assert(newLogicalPlan != logicalPlan,

Review comment:
       why this assert is removed?




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33671:
URL: https://github.com/apache/spark/pull/33671#discussion_r687033338



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
##########
@@ -4215,6 +4215,255 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-36447: non-deterministic CTE dedup") {

Review comment:
       and run it twice with AQE on and off




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897890887


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46899/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894464622


   **[Test build #142172 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142172/testReport)** for PR 33671 at commit [`4f24104`](https://github.com/apache/spark/commit/4f24104e2af4ed07dbaa004e5e15ebc1e92cf2de).
    * This patch **fails to build**.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `case class UnresolvedWith(`
     * `case class CTERelationDef(child: LogicalPlan, id: Long = CTERelationDef.newId) extends UnaryNode `
     * `case class CTERelationRef(`
     * `case class WithCTE(plan: LogicalPlan, cteDefs: Seq[CTERelationDef]) extends LogicalPlan `


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894556660


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142175/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897725942


   **[Test build #142386 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142386/testReport)** for PR 33671 at commit [`3746d76`](https://github.com/apache/spark/commit/3746d76528257c2651a6e3834e7f764921c6bb17).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894463690


   **[Test build #142175 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142175/testReport)** for PR 33671 at commit [`4d08307`](https://github.com/apache/spark/commit/4d08307113e8327c32e394b94164ee51ddb58d22).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897731041


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142386/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maryannxue commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
maryannxue commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894469828


   cc @peter-toth @cloud-fan @sigmod 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897831294


   **[Test build #142393 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142393/testReport)** for PR 33671 at commit [`4cc52f7`](https://github.com/apache/spark/commit/4cc52f74ab8a12599b74ae163e8f4065a99e359d).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897899666


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46899/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894556660


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/142175/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894465594


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46684/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897769111


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46894/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894563038


   **[Test build #142176 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142176/testReport)** for PR 33671 at commit [`7ab352a`](https://github.com/apache/spark/commit/7ab352adf974b25259cf9c5cf8112017d09b0d76).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-897217309


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46858/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894489103


   **[Test build #142176 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/142176/testReport)** for PR 33671 at commit [`7ab352a`](https://github.com/apache/spark/commit/7ab352adf974b25259cf9c5cf8112017d09b0d76).


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-894489549


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46687/
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33671: [SPARK-36447][SQL] Avoid inlining non-deterministic With-CTEs

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33671:
URL: https://github.com/apache/spark/pull/33671#issuecomment-896159502






-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org