You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "peter-toth (via GitHub)" <gi...@apache.org> on 2023/04/11 17:39:09 UTC
[GitHub] [spark] peter-toth opened a new pull request, #40744: [WIP][SPARK-24497][SQL] Support recursive SQL
peter-toth opened a new pull request, #40744:
URL: https://github.com/apache/spark/pull/40744
### What changes were proposed in this pull request?
This PR adds recursive query feature to Spark SQL.
A recursive query is defined using the `WITH RECURSIVE` keywords and referring the name of the common table expression within the query.
The implementation complies with SQL standard and follows similar rules to other relational databases:
- A query is made of an anchor followed by a recursive term.
- The anchor terms doesn't contain self reference and it is used to initialize the query.
- The recursive term contains a self reference and it is used to expand the current set of rows with new ones.
- The anchor and recursive terms must be joined with each other by `UNION` or `UNION ALL` operators.
- New rows can only be derived from the newly added rows of the previous iteration (or from the initial set of rows of anchor terms). This limitation implies that recursive references can't be used with some of the joins, aggregations or subqueries.
Please see `cte-recursive.sql` for some examples.
### Why are the changes needed?
Recursive query is an ANSI SQL feature that is useful to process hierarchical data.
### Does this PR introduce _any_ user-facing change?
Yes, adds recursive query feature.
### How was this patch tested?
Added new UTs and tests in `cte-recursion.sql`.
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-2360412725
Let me close this PR as seemingly it's open state causes some confusion.
Feel free to use reuse the code if anyone wants to tacke this issue.
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "firstim (via GitHub)" <gi...@apache.org>.
firstim commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1981566986
try this when this feature is not available yet.
https://pypi.org/project/pyspark-connectby/
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth closed pull request #40744: [SPARK-24497][SQL] Support recursive SQL
URL: https://github.com/apache/spark/pull/40744
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "SvenRelijveld1995 (via GitHub)" <gi...@apache.org>.
SvenRelijveld1995 commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-2083502536
Any update on this PR?
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "KamilKandzia (via GitHub)" <gi...@apache.org>.
KamilKandzia commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1848946731
@peter-toth Hi, we are very much expecting a recursive sql. We hope you will be able to complete this pull request :)
--
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 #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1568607994
> Thanks @peter-toth. I tested this patch locally. But it seem it throws `StackOverflowError`. How to reproduce:
>
> ```
> ./dev/make-distribution.sh --tgz -Phive -Phive-thriftserver
> tar -zxf spark-3.5.0-SNAPSHOT-bin-3.3.5.tgz
> cd spark-3.5.0-SNAPSHOT-bin-3.3.5
> bin/spark-sql
> ```
>
> ```
> spark-sql (default)> WITH RECURSIVE t(n) AS (
> > VALUES (1)
> > UNION ALL
> > SELECT n+1 FROM t WHERE n < 100
> > )
> > SELECT sum(n) FROM t;
> 23/05/30 13:21:21 ERROR Executor: Exception in task 0.0 in stage 265.0 (TID 199)
> java.lang.StackOverflowError
> at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
> ```
Thanks for testing this PR @wangyum. Iterestingly, I didn't encounter stack overflow when recursion level is <100. The error starts to appear at level ~170 in my tests. I guess this depends on your default stack size. Since recursion works in a way that each iteration depends on the previous iteration the RDD lineage of the tasks are getting bigger and bigger and the deserialization of those tasks can throw stack overflow error at some point. Let me amend this PR with adding optional checkpointing so as to truncate RDD linage and be able to deal with deeper recursion...
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "jeremyjh (via GitHub)" <gi...@apache.org>.
jeremyjh commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-2102832537
@milastdbx are you still planning to take this up?
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "sb-mirakl (via GitHub)" <gi...@apache.org>.
sb-mirakl commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-2360224449
Any update ? Thanks !
--
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] wangyum commented on pull request #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1567776917
Thanks @peter-toth. I tested this patch locally. But it seem it throws `StackOverflowError`.
How to reproduce:
```
./dev/make-distribution.sh --tgz -Phive -Phive-thriftserver
tar -zxf spark-3.5.0-SNAPSHOT-bin-3.3.5.tgz
cd spark-3.5.0-SNAPSHOT-bin-3.3.5
bin/spark-sql
```
```
spark-sql (default)> WITH RECURSIVE t(n) AS (
> VALUES (1)
> UNION ALL
> SELECT n+1 FROM t WHERE n < 100
> )
> SELECT sum(n) FROM t;
23/05/30 13:21:21 ERROR Executor: Exception in task 0.0 in stage 265.0 (TID 199)
java.lang.StackOverflowError
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
```
--
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 diff in pull request #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #40744:
URL: https://github.com/apache/spark/pull/40744#discussion_r1213420095
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -714,6 +717,121 @@ case class UnionExec(children: Seq[SparkPlan]) extends SparkPlan {
copy(children = newChildren)
}
+/**
+ * The physical node for recursion.
+ *
+ * @param loopId The id of the loop.
+ * @param anchor The logical plan of the initial element of the loop.
+ * @param recursion The logical plan that describes the recursion with an [[UnionLoopRef]] node.
+ * @param output The output attributes of this loop.
+ * @param limit An optional limit that can be pushed down to the node to stop the loop earlier.
+ */
+case class UnionLoopExec(
+ loopId: Long,
+ @transient anchor: LogicalPlan,
+ @transient recursion: LogicalPlan,
+ override val output: Seq[Attribute],
+ limit: Option[Int] = None) extends LeafExecNode {
+
+ val levelLimit = conf.getConf(SQLConf.CTE_RECURSION_LEVEL_LIMIT)
+
+ // We store the initial and generated children of the loop in this buffer.
+ // Please note that all elements are cached because of performance reasons as they are needed for
+ // next iteration.
+ @transient private val unionDFs = mutable.ArrayBuffer.empty[DataFrame]
+
+ override def innerChildren: Seq[QueryPlan[_]] = Seq(anchor, recursion)
+
+ override lazy val metrics = Map(
+ "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of output rows"))
+
+ private def cacheAndCount(plan: LogicalPlan, limit: Option[Long]) = {
+ val limitedPlan = limit.map(l => Limit(Literal(l.toInt), plan)).getOrElse(plan)
+ val df = Dataset.ofRows(session, limitedPlan).persist()
Review Comment:
`repartition()` seems to be good option to truncate RDD lineage and decrease task sizes to avoid stack overflow. I added it as the default cache mode in https://github.com/apache/spark/pull/40744/commits/2c206a081acee51b4cf7d375a2801fbf6f8b67cc.
--
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] wangyum commented on a diff in pull request #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40744:
URL: https://github.com/apache/spark/pull/40744#discussion_r1212539620
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -714,6 +717,121 @@ case class UnionExec(children: Seq[SparkPlan]) extends SparkPlan {
copy(children = newChildren)
}
+/**
+ * The physical node for recursion.
+ *
+ * @param loopId The id of the loop.
+ * @param anchor The logical plan of the initial element of the loop.
+ * @param recursion The logical plan that describes the recursion with an [[UnionLoopRef]] node.
+ * @param output The output attributes of this loop.
+ * @param limit An optional limit that can be pushed down to the node to stop the loop earlier.
+ */
+case class UnionLoopExec(
+ loopId: Long,
+ @transient anchor: LogicalPlan,
+ @transient recursion: LogicalPlan,
+ override val output: Seq[Attribute],
+ limit: Option[Int] = None) extends LeafExecNode {
+
+ val levelLimit = conf.getConf(SQLConf.CTE_RECURSION_LEVEL_LIMIT)
+
+ // We store the initial and generated children of the loop in this buffer.
+ // Please note that all elements are cached because of performance reasons as they are needed for
+ // next iteration.
+ @transient private val unionDFs = mutable.ArrayBuffer.empty[DataFrame]
+
+ override def innerChildren: Seq[QueryPlan[_]] = Seq(anchor, recursion)
+
+ override lazy val metrics = Map(
+ "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of output rows"))
+
+ private def cacheAndCount(plan: LogicalPlan, limit: Option[Long]) = {
+ val limitedPlan = limit.map(l => Limit(Literal(l.toInt), plan)).getOrElse(plan)
+ val df = Dataset.ofRows(session, limitedPlan).persist()
+ val count = df.count()
+ (df, count)
+ }
+
+ override protected def doExecute(): RDD[InternalRow] = {
+ val executionId = sparkContext.getLocalProperty(SQLExecution.EXECUTION_ID_KEY)
+ val numOutputRows = longMetric("numOutputRows")
+
+ val unionChildren = mutable.ArrayBuffer.empty[LogicalRDD]
+ var currentLimit = limit.map(_.toLong)
+ var (prevDF, prevCount) = cacheAndCount(anchor, currentLimit)
+
+ var currentLevel = 0
Review Comment:
Why `currentLevel` is 0, not 1?
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "waywtdcc (via GitHub)" <gi...@apache.org>.
waywtdcc commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1975541287
Can this PR be merged? I also encountered this scenario
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "milimetric (via GitHub)" <gi...@apache.org>.
milimetric commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1778014631
@peter-toth thank you so much for sticking with this over three major versions and three separate pull requests. Recursive queries would be really nice to have in Spark SQL.
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "milastdbx (via GitHub)" <gi...@apache.org>.
milastdbx commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1869048522
Yes, thank you.
Milan
On Thu, Dec 21, 2023 at 12:29 PM Peter Toth ***@***.***>
wrote:
> @milastdbx <https://github.com/milastdbx> do you think you can take over
> this PR?
>
> cc @cloud-fan <https://github.com/cloud-fan>, @mitkedb
> <https://github.com/mitkedb>
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/spark/pull/40744#issuecomment-1866092645>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BD3GPBCAU24GF5QGXTQCYLLYKQMRHAVCNFSM6AAAAAAW2SUTGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRWGA4TENRUGU>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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 diff in pull request #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #40744:
URL: https://github.com/apache/spark/pull/40744#discussion_r1213420095
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -714,6 +717,121 @@ case class UnionExec(children: Seq[SparkPlan]) extends SparkPlan {
copy(children = newChildren)
}
+/**
+ * The physical node for recursion.
+ *
+ * @param loopId The id of the loop.
+ * @param anchor The logical plan of the initial element of the loop.
+ * @param recursion The logical plan that describes the recursion with an [[UnionLoopRef]] node.
+ * @param output The output attributes of this loop.
+ * @param limit An optional limit that can be pushed down to the node to stop the loop earlier.
+ */
+case class UnionLoopExec(
+ loopId: Long,
+ @transient anchor: LogicalPlan,
+ @transient recursion: LogicalPlan,
+ override val output: Seq[Attribute],
+ limit: Option[Int] = None) extends LeafExecNode {
+
+ val levelLimit = conf.getConf(SQLConf.CTE_RECURSION_LEVEL_LIMIT)
+
+ // We store the initial and generated children of the loop in this buffer.
+ // Please note that all elements are cached because of performance reasons as they are needed for
+ // next iteration.
+ @transient private val unionDFs = mutable.ArrayBuffer.empty[DataFrame]
+
+ override def innerChildren: Seq[QueryPlan[_]] = Seq(anchor, recursion)
+
+ override lazy val metrics = Map(
+ "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of output rows"))
+
+ private def cacheAndCount(plan: LogicalPlan, limit: Option[Long]) = {
+ val limitedPlan = limit.map(l => Limit(Literal(l.toInt), plan)).getOrElse(plan)
+ val df = Dataset.ofRows(session, limitedPlan).persist()
Review Comment:
`repartition()` seems to be good option to truncate RDD lineage and decrease task sizes to avoid stack overflow. I added it as the default cache mode in https://github.com/apache/spark/pull/40744/commits/02f527de85fbb1f5930b5f044df7edeef2f255ee.
--
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] wangyum commented on a diff in pull request #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40744:
URL: https://github.com/apache/spark/pull/40744#discussion_r1212534490
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -714,6 +717,121 @@ case class UnionExec(children: Seq[SparkPlan]) extends SparkPlan {
copy(children = newChildren)
}
+/**
+ * The physical node for recursion.
+ *
+ * @param loopId The id of the loop.
+ * @param anchor The logical plan of the initial element of the loop.
+ * @param recursion The logical plan that describes the recursion with an [[UnionLoopRef]] node.
+ * @param output The output attributes of this loop.
+ * @param limit An optional limit that can be pushed down to the node to stop the loop earlier.
+ */
+case class UnionLoopExec(
+ loopId: Long,
+ @transient anchor: LogicalPlan,
+ @transient recursion: LogicalPlan,
+ override val output: Seq[Attribute],
+ limit: Option[Int] = None) extends LeafExecNode {
+
+ val levelLimit = conf.getConf(SQLConf.CTE_RECURSION_LEVEL_LIMIT)
+
+ // We store the initial and generated children of the loop in this buffer.
+ // Please note that all elements are cached because of performance reasons as they are needed for
+ // next iteration.
+ @transient private val unionDFs = mutable.ArrayBuffer.empty[DataFrame]
+
+ override def innerChildren: Seq[QueryPlan[_]] = Seq(anchor, recursion)
+
+ override lazy val metrics = Map(
+ "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of output rows"))
+
+ private def cacheAndCount(plan: LogicalPlan, limit: Option[Long]) = {
+ val limitedPlan = limit.map(l => Limit(Literal(l.toInt), plan)).getOrElse(plan)
+ val df = Dataset.ofRows(session, limitedPlan).persist()
Review Comment:
Could we replace `persist()` with `repartition()` to avoid stack overflow issue?
--
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 #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1623203290
> Hey folks, So glad to see this feature is being worked on. Do you have any estimates when this could be released ?
This feature very likely won't make it into the next release (Spark 3.5) as tbe branch cut is in 2 weeks. But I will try to add it to one after the next (Spark 4.0).
--
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 #40744: [WIP][SPARK-24497][SQL] Support recursive SQL
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1515969429
This PR is WIP as it contains https://github.com/apache/spark/pull/40856. Once that PR is merged I will rebase and remove the WIP flag.
--
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 #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1525540641
https://github.com/apache/spark/pull/40856 got merged and I've rebased this PR. I'm removing the WIP flag and the PR is ready for review.
cc @cloud-fan, @wangyum, @maryannxue, @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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "jboarman (via GitHub)" <gi...@apache.org>.
jboarman commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-2161488091
@wangyum I see that you started the review last year and the issues you raised were addressed by Peter.
Then @milastdbx was tagged to take over the PR, but I don't see the issue being assigned to you yet.
How do we get this PR reviewed?
cc @cloud-fan, @mitkedb, @MaxGekk
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "travis-leith (via GitHub)" <gi...@apache.org>.
travis-leith commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-2205869472
> > Thanks @peter-toth. I tested this patch locally. But it seem it throws `StackOverflowError`. How to reproduce:
> > ```
> > ./dev/make-distribution.sh --tgz -Phive -Phive-thriftserver
> > tar -zxf spark-3.5.0-SNAPSHOT-bin-3.3.5.tgz
> > cd spark-3.5.0-SNAPSHOT-bin-3.3.5
> > bin/spark-sql
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > ```
> > spark-sql (default)> WITH RECURSIVE t(n) AS (
> > > VALUES (1)
> > > UNION ALL
> > > SELECT n+1 FROM t WHERE n < 100
> > > )
> > > SELECT sum(n) FROM t;
> > 23/05/30 13:21:21 ERROR Executor: Exception in task 0.0 in stage 265.0 (TID 199)
> > java.lang.StackOverflowError
> > at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
> > ```
>
> Thanks for testing this PR @wangyum. Iterestingly, I didn't encounter stack overflow when recursion level is <100. The error starts to appear at level ~170 in my tests. I guess this depends on your default stack size. Since recursion works in a way that each iteration depends on the previous iteration, the RDD lineage of the tasks are getting bigger and bigger and the deserialization of those tasks can throw stack overflow error at some point. Let me amend this PR with adding optional checkpointing so as to truncate RDD linage and be able to deal with deeper recursion...
@peter-toth I have not looked closely at the implementation but I do have a question about this: has the logic been implemented in some way similar to tail call optimization such that there is no recursion depth limit?
--
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] ksn06 commented on pull request #40744: [SPARK-24497][SQL] Support recursive SQL
Posted by "ksn06 (via GitHub)" <gi...@apache.org>.
ksn06 commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1623162180
Hey folks,
So glad to see this feature is being worked on. Do you have any estimates when this could be released ?
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1763217582
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-1866092645
@milastdbx do you think you can take over this PR?
cc @cloud-fan, @mitkedb
--
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
Re: [PR] [SPARK-24497][SQL] Support recursive SQL [spark]
Posted by "jeremyjh (via GitHub)" <gi...@apache.org>.
jeremyjh commented on PR #40744:
URL: https://github.com/apache/spark/pull/40744#issuecomment-2370906873
@peter-toth can you also update the status on the Jira ticket? https://issues.apache.org/jira/browse/SPARK-24497
--
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