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 2022/11/05 17:56:11 UTC

[GitHub] [spark] MaxGekk opened a new pull request, #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`

MaxGekk opened a new pull request, #38521:
URL: https://github.com/apache/spark/pull/38521

   ### What changes were proposed in this pull request?
   In the PR, I propose to rename the legacy error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`.
   
   ### Why are the changes needed?
   Proper names of error classes should improve user experience with Spark SQL.
   
   ### Does this PR introduce _any_ user-facing change?
   Yes.
   
   ### How was this patch tested?
   By running the affected test suites:
   ```
   $ build/sbt "core/testOnly *SparkThrowableSuite"
   $ PYSPARK_PYTHON=python3 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z group-by-ordinal.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


[GitHub] [spark] MaxGekk commented on pull request #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #38521:
URL: https://github.com/apache/spark/pull/38521#issuecomment-1304724265

   @cloud-fan @srielau @itholic @LuciferYang @panbingkun Please, review 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


[GitHub] [spark] MaxGekk commented on pull request #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #38521:
URL: https://github.com/apache/spark/pull/38521#issuecomment-1305280424

   Merging to master. Thank you, @cloud-fan @LuciferYang @itholic @srielau.


-- 
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] srielau commented on a diff in pull request #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`

Posted by GitBox <gi...@apache.org>.
srielau commented on code in PR #38521:
URL: https://github.com/apache/spark/pull/38521#discussion_r1014863229


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -912,6 +912,11 @@
     ],
     "sqlState" : "22023"
   },
+  "STAR_GROUP_BY_POS" : {

Review Comment:
   LGTM ... but why do we have this restriction?



-- 
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] MaxGekk commented on a diff in pull request #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #38521:
URL: https://github.com/apache/spark/pull/38521#discussion_r1014871999


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -912,6 +912,11 @@
     ],
     "sqlState" : "22023"
   },
+  "STAR_GROUP_BY_POS" : {

Review Comment:
   It is better to ask the author of the restriction, cc @gatorsmile and the committer @cloud-fan 
   https://github.com/apache/spark/pull/11846/files#diff-ed19f376a63eba52eea59ca71f3355d4495fad4fad4db9a3324aade0d4986a47R389-R392



-- 
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] MaxGekk commented on a diff in pull request #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #38521:
URL: https://github.com/apache/spark/pull/38521#discussion_r1014871999


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -912,6 +912,11 @@
     ],
     "sqlState" : "22023"
   },
+  "STAR_GROUP_BY_POS" : {

Review Comment:
   It is better to ask the author of the restriction, cc @gatorsmile 
   https://github.com/apache/spark/pull/11846/files#diff-ed19f376a63eba52eea59ca71f3355d4495fad4fad4db9a3324aade0d4986a47R389-R392



-- 
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] MaxGekk closed pull request #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`

Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #38521: [SPARK-41020][SQL] Rename the error class `_LEGACY_ERROR_TEMP_1019` to `STAR_GROUP_BY_POS`
URL: https://github.com/apache/spark/pull/38521


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