You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "valentinp17 (via GitHub)" <gi...@apache.org> on 2023/08/28 21:25:06 UTC
[GitHub] [spark] valentinp17 opened a new pull request, #42706: [SPARK-42304][SQL] Assign name to _LEGACY_ERROR_TEMP_2189
valentinp17 opened a new pull request, #42706:
URL: https://github.com/apache/spark/pull/42706
### What changes were proposed in this pull request?
This PR proposes to assign name to _LEGACY_ERROR_TEMP_2189, "UNSUPPORTED_FUNCTION_BY_HIVE_VERSION".
### Why are the changes needed?
Assign proper name to LEGACY_ERROR_TEMP*
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
./build/sbt "testOnly org.apache.spark.SparkThrowableSuite"
### Was this patch authored or co-authored using generative AI tooling?
No
--
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] valentinp17 commented on pull request #42706: [SPARK-42304][SQL] Assign name to _LEGACY_ERROR_TEMP_2189
Posted by "valentinp17 (via GitHub)" <gi...@apache.org>.
valentinp17 commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1696445360
@MaxGekk @itholic PTAL when you find some time.
--
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] valentinp17 commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "valentinp17 (via GitHub)" <gi...@apache.org>.
valentinp17 commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1712960745
@itholic Hi!
Of course, I'm interested in!
I'm unsure whether I should create a new Jira for these changes or not. So I decided to show tests in my fork repo first.
PTAL https://github.com/valentinp17/spark/commit/d44216b776ae070a3fcace0c31e5f367513a0c30
cc @MaxGekk @cxzl25
--
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] itholic commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1699634991
Welcome, @valentinp17 !
--
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] valentinp17 commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "valentinp17 (via GitHub)" <gi...@apache.org>.
valentinp17 commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1698619271
> Do you have ASF JIRA account, @valentinp17 ?
>
> I can assign https://issues.apache.org/jira/browse/SPARK-42304 to you if you have the ID.
Not yet. I sent a request to create ASF JIRA account.
Can I tag you here to assign this Jira ticket later?
--
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 #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
URL: https://github.com/apache/spark/pull/42706
--
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] valentinp17 commented on pull request #42706: [SPARK-42304][SQL] Assign name to _LEGACY_ERROR_TEMP_2189
Posted by "valentinp17 (via GitHub)" <gi...@apache.org>.
valentinp17 commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1697154005
> Can we add a dedicated test for this error class??
I will try to do it
I think this test should cover getTablesByType() https://github.com/apache/spark/blob/76a32d3df0fe8e857221662657936e5ae336d3ee/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala#L1631
--
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] valentinp17 commented on a diff in pull request #42706: [SPARK-42304][SQL] Assign name to _LEGACY_ERROR_TEMP_2189
Posted by "valentinp17 (via GitHub)" <gi...@apache.org>.
valentinp17 commented on code in PR #42706:
URL: https://github.com/apache/spark/pull/42706#discussion_r1308507702
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -3205,6 +3205,11 @@
},
"sqlState" : "0A000"
},
+ "UNSUPPORTED_FUNCTION_BY_HIVE_VERSION" : {
+ "message" : [
+ "Hive 2.2 and lower versions don't support getTablesByType. Please use Hive 2.3 or higher version."
Review Comment:
I guess I agree.
I can rename to "GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION" if it's not too long
--
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] valentinp17 commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "valentinp17 (via GitHub)" <gi...@apache.org>.
valentinp17 commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1699566201
Thank you, @dongjoon-hyun
I created ASF JIRA account. Jira username: valentinp17
--
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] itholic commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1713045646
> I'm unsure whether I should create a new Jira for these changes or not
We can just reuse the existing JIRA, or we can create a new one. This usually depends on the size of the task, but it's totally up to you :-).
In case reusing the existing JIRA, we typically add a `[FOLLOWUP]` tag, such as "[SPARK-42304][FOLLOWUP][SQL] Add test for `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`"
> I suppose it should link to 'common/utils/src/main/resources/error/README.md' instead. If I'm right, I can create PR for this too.
Yeah, seems the link is broken. Let's fix 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] dongjoon-hyun commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1699617223
I added to you the Apache Spark contributor group and assigned SPARK-42304 to you.
Welcome to the Apache Spark community, @valentinp17 !
--
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 #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1698481948
Do you have ASF JIRA account, @valentinp17 ?
I can assign https://issues.apache.org/jira/browse/SPARK-42304 to you if you have the 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] valentinp17 commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "valentinp17 (via GitHub)" <gi...@apache.org>.
valentinp17 commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1712960810
Additional observation:
Pull request template message looks outdated:
> 8. If you want to add or modify an error type or message, please read the guideline first in 'core/src/main/resources/error/README.md'
I suppose it should link to 'common/utils/src/main/resources/error/README.md' instead. If I'm right, I can create PR for this too.
--
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] itholic commented on a diff in pull request #42706: [SPARK-42304][SQL] Assign name to _LEGACY_ERROR_TEMP_2189
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42706:
URL: https://github.com/apache/spark/pull/42706#discussion_r1308078838
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -3205,6 +3205,11 @@
},
"sqlState" : "0A000"
},
+ "UNSUPPORTED_FUNCTION_BY_HIVE_VERSION" : {
+ "message" : [
+ "Hive 2.2 and lower versions don't support getTablesByType. Please use Hive 2.3 or higher version."
Review Comment:
Is the `getTablesByType` only function that is not supported by Hive 2.2 and lower?? Otherwise I feel the "FUNCTION" in the error class name might too generic.
--
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] itholic commented on pull request #42706: [SPARK-42304][SQL] Assign name to _LEGACY_ERROR_TEMP_2189
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1696599482
Can we add a dedicated test for this error class??
--
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] itholic commented on pull request #42706: [SPARK-42304][SQL] Rename `_LEGACY_ERROR_TEMP_2189` to `GET_TABLES_BY_TYPE_UNSUPPORTED_BY_HIVE_VERSION`
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #42706:
URL: https://github.com/apache/spark/pull/42706#issuecomment-1701965379
> I will try to do it
> I think this test should cover getTablesByType()
Btw, I believe you can add a test as your next work if you're interested in! :-)
--
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