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/02/16 10:58:15 UTC

[GitHub] [spark] Yikf opened a new pull request #35541: [SPARK-38229][SQL] Should't check temp/external/ifNotExists with visitReplaceTable when parser

Yikf opened a new pull request #35541:
URL: https://github.com/apache/spark/pull/35541


   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
     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'.
   -->
   
   ### What changes were proposed in this pull request?
   Spark does not support replace table syntax such as CREATE OR REPLACE TEMPORARY TABLE.../REPLACE EXTERNAL TABLE/REPLACE ... IF NOT EXISTS, And we don't need to check these tokens
   ![image](https://user-images.githubusercontent.com/51110188/154250690-07a28054-3fc1-4f3e-ad1c-206b02273111.png)
   
   
   ### Why are the changes needed?
   code simplification
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   Exist ut


-- 
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 #35541: [SPARK-38229][SQL] Should't check temp/external/ifNotExists with visitReplaceTable when parser

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


   


-- 
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 #35541: [SPARK-38229][SQL] Should't check temp/external/ifNotExists with visitReplaceTable when parser

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


   thanks, merging 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] Yikf commented on a change in pull request #35541: [SPARK-38229][SQL] Should't check temp/external/ifNotExists with visitReplaceTable when parser

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2945,9 +2945,9 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
    * Validate a replace table statement and return the [[TableIdentifier]].
    */
   override def visitReplaceTableHeader(
-      ctx: ReplaceTableHeaderContext): TableHeader = withOrigin(ctx) {
+      ctx: ReplaceTableHeaderContext): Seq[String] = withOrigin(ctx) {

Review comment:
       Sure, I will file a follow-up for it 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] AmplabJenkins commented on pull request #35541: [SPARK-38229][SQL] Should't check temp/external/ifNotExists with visitReplaceTable when parser

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


   Can one of the admins verify this patch?


-- 
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] Yikf commented on pull request #35541: [SPARK-38229][SQL] Should't check temp/external/ifNotExists with visitReplaceTable when parser

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


   @cloud-fan Could you please take a look, 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] cloud-fan commented on a change in pull request #35541: [SPARK-38229][SQL] Should't check temp/external/ifNotExists with visitReplaceTable when parser

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



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2945,9 +2945,9 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
    * Validate a replace table statement and return the [[TableIdentifier]].
    */
   override def visitReplaceTableHeader(
-      ctx: ReplaceTableHeaderContext): TableHeader = withOrigin(ctx) {
+      ctx: ReplaceTableHeaderContext): Seq[String] = withOrigin(ctx) {

Review comment:
       Can we further clean it up? Now this method simply parses the table name, and we don't need to have a `visitReplaceTableHeader` method for 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