You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jchen5 (via GitHub)" <gi...@apache.org> on 2023/05/17 16:02:14 UTC
[GitHub] [spark] jchen5 opened a new pull request, #41202: [SPARK-43413][SQL] Mention flag in assert error message for ListQuery nullable
jchen5 opened a new pull request, #41202:
URL: https://github.com/apache/spark/pull/41202
### What changes were proposed in this pull request?
In case the assert for the call to ListQuery.nullable is hit, mention in the assert error message the conf flag that can be used to disable the assert. Follow-up to https://github.com/apache/spark/pull/41094#discussion_r1195438179
### Why are the changes needed?
Improve error message.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit tests
--
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] dongjoon-hyun commented on a diff in pull request #41202: [SPARK-43413][SQL][FOLLOWUP] Show a directional message in ListQuery nullability assertion
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41202:
URL: https://github.com/apache/spark/pull/41202#discussion_r1196870894
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/subquery.scala:
##########
@@ -372,7 +372,8 @@ case class ListQuery(
// ListQuery can't be executed alone so its nullability is not defined.
// Consider using ListQuery.childOutputs.exists(_.nullable)
if (!SQLConf.get.getConf(SQLConf.LEGACY_IN_SUBQUERY_NULLABILITY)) {
- assert(false, "ListQuery nullability is not defined")
+ assert(false, "ListQuery nullability is not defined. To restore the legacy behavior before " +
+ "Spark 3.5.0, set " + SQLConf.LEGACY_IN_SUBQUERY_NULLABILITY.key + " = true")
Review Comment:
nit. We prefer the following style.
```scala
- "Spark 3.5.0, set " + SQLConf.LEGACY_IN_SUBQUERY_NULLABILITY.key + " = true"
+ s"Spark 3.5.0, set ${SQLConf.LEGACY_IN_SUBQUERY_NULLABILITY.key}=true"
```
--
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] dongjoon-hyun commented on a diff in pull request #41202: [SPARK-43413][SQL] Mention flag in assert error message for ListQuery nullable
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41202:
URL: https://github.com/apache/spark/pull/41202#discussion_r1196850103
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/subquery.scala:
##########
@@ -372,7 +372,8 @@ case class ListQuery(
// ListQuery can't be executed alone so its nullability is not defined.
// Consider using ListQuery.childOutputs.exists(_.nullable)
if (!SQLConf.get.getConf(SQLConf.LEGACY_IN_SUBQUERY_NULLABILITY)) {
- assert(false, "ListQuery nullability is not defined")
+ assert(false, "ListQuery nullability is not defined. This assert can be disabled using " +
Review Comment:
```
- This assert can be disabled using conf ...
+ To restore the behavior before Spark 3.5.0, set ...
```
--
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] jchen5 commented on pull request #41202: [SPARK-43413][SQL][FOLLOWUP] Show a directional message in ListQuery nullability assertion
Posted by "jchen5 (via GitHub)" <gi...@apache.org>.
jchen5 commented on PR #41202:
URL: https://github.com/apache/spark/pull/41202#issuecomment-1551806149
Thanks for comments, updated
--
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] dongjoon-hyun commented on pull request #41202: [SPARK-43413][SQL][FOLLOWUP] Show a directional message in ListQuery nullability assertion
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #41202:
URL: https://github.com/apache/spark/pull/41202#issuecomment-1552472727
Merged to master~
--
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] dongjoon-hyun closed pull request #41202: [SPARK-43413][SQL][FOLLOWUP] Show a directional message in ListQuery nullability assertion
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #41202: [SPARK-43413][SQL][FOLLOWUP] Show a directional message in ListQuery nullability assertion
URL: https://github.com/apache/spark/pull/41202
--
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] dongjoon-hyun commented on a diff in pull request #41202: [SPARK-43413][SQL] Mention flag in assert error message for ListQuery nullable
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41202:
URL: https://github.com/apache/spark/pull/41202#discussion_r1196848013
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/subquery.scala:
##########
@@ -372,7 +372,8 @@ case class ListQuery(
// ListQuery can't be executed alone so its nullability is not defined.
// Consider using ListQuery.childOutputs.exists(_.nullable)
if (!SQLConf.get.getConf(SQLConf.LEGACY_IN_SUBQUERY_NULLABILITY)) {
- assert(false, "ListQuery nullability is not defined")
+ assert(false, "ListQuery nullability is not defined. This assert can be disabled using " +
+ "conf spark.sql.legacy.inSubqueryNullability = true")
Review Comment:
Instead of a hard-coded string, we use `SQLConf.LEGACY_IN_SUBQUERY_NULLABILITY.key`.
--
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