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/10/28 22:48:47 UTC
[GitHub] [spark] ben-zhang opened a new pull request, #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
ben-zhang opened a new pull request, #38433:
URL: https://github.com/apache/spark/pull/38433
<!--
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?
Make the `MSCK` keyword optional in `MSCK REPAIR TABLE` commands so that it can be omitted.
### Why are the changes needed?
The use of the keyword `MSCK`, meaning metastore check, is arcane and does not add value for the command. Removing it makes the meaning of `REPAIR TABLE` commands more clear.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
Yes, previously, it was no possible to specify only `REPAIR TABLE` without `MSCK`. Now, it is possible. No changes to existing behaviour using the original `MSCK REPAIR TABLE` syntax are in this PR.
### 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] ben-zhang commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
ben-zhang commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1297443974
@MaxGekk 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 #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1297475005
cc @sunchao since he is the Hive PMC member and Spark committer.
--
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 diff in pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #38433:
URL: https://github.com/apache/spark/pull/38433#discussion_r1009998364
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -1728,6 +1728,14 @@ class DataSourceV2SQLSuiteV1Filter extends DataSourceV2SQLSuite with AlterTableT
}
}
+ test("REPAIR TABLE omit MSCK") {
Review Comment:
can we move the test to `MsckRepairTableParserSuite`? And probably rename the test suite. And also `MsckRepairTableSuiteBase` and sub classes.
--
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] ben-zhang commented on a diff in pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
ben-zhang commented on code in PR #38433:
URL: https://github.com/apache/spark/pull/38433#discussion_r1010063647
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -1728,6 +1728,14 @@ class DataSourceV2SQLSuiteV1Filter extends DataSourceV2SQLSuite with AlterTableT
}
}
+ test("REPAIR TABLE omit MSCK") {
Review Comment:
Sounds good, I can remove this one. I agree that e2e tests are not necessary for a parser change.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1416536574
Also cc @bjornjorgensen , @cloud-fan , @MaxGekk , and @xinrong-meng for Aapche Spark 3.4 release manager.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1423797387
@ben-zhang . Could you address @MaxGekk 's review comment?
--
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] ben-zhang commented on pull request #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "ben-zhang (via GitHub)" <gi...@apache.org>.
ben-zhang commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432616439
@MaxGekk @dongjoon-hyun agreed, the inconsistency with error messages is strange. I will create a follow-up for that.
--
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 diff in pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #38433:
URL: https://github.com/apache/spark/pull/38433#discussion_r1009998588
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -1728,6 +1728,14 @@ class DataSourceV2SQLSuiteV1Filter extends DataSourceV2SQLSuite with AlterTableT
}
}
+ test("REPAIR TABLE omit MSCK") {
Review Comment:
oh this is not parser test. But I think a parser change only needs parser test, no end-to-end test.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432604300
> Sorry, but let's revert your last commit back and merge this PR without addressing your comments. Is it okay, @MaxGekk ?
I just don't like the consistency between the docs where we talk only about `REPAIR TABLE` and the error message that will seems outdated after the PR. But we can make the changes later, for sure.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432598306
@dongjoon-hyun Need to change this one too:
```scala
RepairTable(
createUnresolvedTable(ctx.multipartIdentifier, s"MSCK REPAIR TABLE$option"),
```
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432619979
Merged to master/3.4 for Apache Spark 3.4.
Thank you, @ben-zhang , @cloud-fan , @MaxGekk , @bjornjorgensen , @srielau .
Also, I added you to the Apache Spark contributor group and assigned SPARK-40943 to you, @ben-zhang .
Welcome to the Apache Spark community (OSS).
--
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] ben-zhang commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in REPAIR TABLE commands
Posted by "ben-zhang (via GitHub)" <gi...@apache.org>.
ben-zhang commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1416399563
Hey @dongjoon-hyun, you can see the updated docks here: https://docs.databricks.com/sql/language-manual/sql-ref-syntax-ddl-repair-table.html
Shall we revisit this idea?
--
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] ben-zhang commented on pull request #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "ben-zhang (via GitHub)" <gi...@apache.org>.
ben-zhang commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1416542173
Thanks for being so prompt on the revival of this PR, @dongjoon-hyun. Appreciate 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] ben-zhang commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
ben-zhang commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1296345928
> Could you provide other DBMS references for `REPAIR TABLE` syntax without `MSCK`, @ben-zhang ?
Hey @dongjoon-hyun, thanks for asking.
Here's a [reference from MYSQL](https://dev.mysql.com/doc/refman/8.0/en/repair-table.html).
Also, the Spark documentation page for this command is titled "REPAIR TABLE". It only mentions the MSCK keyword in the content of the page. To me, this is an acknowledgement that the meaning of the command is tied to just "REPAIR TABLE".
--
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 #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1297804976
May I ask if dropping `MSCK` is the official Databricks's direction in order to do the lead, @srielau ?
> As perhaps the most popular platform, we have the chance to lead here and make things easier.
--
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 pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
srielau commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1297817456
> May I ask if dropping `MSCK` is the official Databricks's direction in order to do the lead, @srielau ?
>
> > As perhaps the most popular platform, we have the chance to lead here and make things easier.
Yes, Databricks will make MSCK a noise word. I will probably remove it from our documented syntax and add a footnote such as "for compatibility with Hive you may also use MSCK REPAIR TABLE".
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #38433:
URL: https://github.com/apache/spark/pull/38433#discussion_r1096514852
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/MsckRepairTableParserSuite.scala:
##########
@@ -31,6 +31,15 @@ class MsckRepairTableParserSuite extends AnalysisTest {
enableDropPartitions = false))
}
+ test("repair a table without MSCK keyword") {
+ comparePlans(
+ parsePlan("REPAIR TABLE a.b.c"),
+ RepairTable(
+ UnresolvedTable(Seq("a", "b", "c"), "MSCK REPAIR TABLE", None),
Review Comment:
Shouldn't we change the hint if `REPAIR TABLE` becomes the main way of invoking the command?
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38433:
URL: https://github.com/apache/spark/pull/38433#discussion_r1108062600
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/MsckRepairTableParserSuite.scala:
##########
@@ -31,6 +31,15 @@ class MsckRepairTableParserSuite extends AnalysisTest {
enableDropPartitions = false))
}
+ test("repair a table without MSCK keyword") {
+ comparePlans(
+ parsePlan("REPAIR TABLE a.b.c"),
+ RepairTable(
+ UnresolvedTable(Seq("a", "b", "c"), "MSCK REPAIR TABLE", None),
Review Comment:
According to the agreement here, I'll merge @MaxGekk 's suggestion and backport to branch-3.4 for Apache Spark 3.4 release.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432601531
Sorry, but let's revert your last commit back and merge this PR without addressing your comments. Is it okay, @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
[GitHub] [spark] AmplabJenkins commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1296050675
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] cloud-fan commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1297895255
Making `MSCK` optional makes our SQL syntax more flexible (and simpler), and compatible with MySQL. And there seems no down side. @dongjoon-hyun Can you share your concerns? We can discuss more here.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #38433:
URL: https://github.com/apache/spark/pull/38433#discussion_r1108056183
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/MsckRepairTableParserSuite.scala:
##########
@@ -31,6 +31,15 @@ class MsckRepairTableParserSuite extends AnalysisTest {
enableDropPartitions = false))
}
+ test("repair a table without MSCK keyword") {
+ comparePlans(
+ parsePlan("REPAIR TABLE a.b.c"),
+ RepairTable(
+ UnresolvedTable(Seq("a", "b", "c"), "MSCK REPAIR TABLE", None),
Review Comment:
```suggestion
UnresolvedTable(Seq("a", "b", "c"), "REPAIR TABLE", None),
```
--
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 #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in REPAIR TABLE commands
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1416470984
Thank you for updates, @ben-zhang . +1 for resuming this PR. Could you rebase this PR, @ben-zhang ?
--
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 pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
srielau commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1297801930
> If MySQL is the only DBMS for this syntax, I'm negative about this change because this PR only increases the chance of SQL incompatibility. Is there any other support cases?
IMHO mySQL was onto something to not include MSCK (who is the chicken and who the egg here anyway?).
It seems like everyone (e.g. IBM) just copied Hive to not rock the boat.
Interestingly our own docs omit MSCK from the title (and the file name):
https://spark.apache.org/docs/3.0.0-preview/sql-ref-syntax-ddl-repair-table.html
It seems the only purpose of this keyword is to trip people up.
As perhaps the most popular platform, we have the chance to lead here and make things easier.
--
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] ben-zhang commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
ben-zhang commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1301158208
@dongjoon-hyun , sounds good. I will get back to you when the Databricks docs are 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] MaxGekk commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1296804585
> Also, the [Spark documentation page](https://spark.apache.org/docs/latest/sql-ref-syntax-ddl-repair-table.html) for this command ...
@ben-zhang Could you update the doc, and mark the `MSCK` keyword as optional one:
Syntax
```
[MSCK] REPAIR TABLE table_identifier [{ADD|DROP|SYNC} PARTITIONS]
```
--
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] ben-zhang commented on a diff in pull request #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "ben-zhang (via GitHub)" <gi...@apache.org>.
ben-zhang commented on code in PR #38433:
URL: https://github.com/apache/spark/pull/38433#discussion_r1102033313
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/MsckRepairTableParserSuite.scala:
##########
@@ -31,6 +31,15 @@ class MsckRepairTableParserSuite extends AnalysisTest {
enableDropPartitions = false))
}
+ test("repair a table without MSCK keyword") {
+ comparePlans(
+ parsePlan("REPAIR TABLE a.b.c"),
+ RepairTable(
+ UnresolvedTable(Seq("a", "b", "c"), "MSCK REPAIR TABLE", None),
Review Comment:
Agreed, this should be updated. Will do this now and then push again.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432597842
Oh, the last commit seems to cause a test failure.
--
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] bjornjorgensen commented on pull request #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "bjornjorgensen (via GitHub)" <gi...@apache.org>.
bjornjorgensen commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1416750618
Yes, I think this is ok. Now that Databrick has it, I hope spark shall also have it the same way.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432601267
> If we touch AstBuilder.scala, it could cause more failures, couldn't it?
Highly likely, all parse/error tests related to `REPAIR TABLE`.
--
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 #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1432600456
If we touch `AstBuilder.scala`, it could cause more failures, couldn't 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 closed pull request #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #38433: [SPARK-40943][SQL] Make `MSCK` keyword optional in `REPAIR TABLE` syntax
URL: https://github.com/apache/spark/pull/38433
--
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] bjornjorgensen commented on pull request #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in MSCK REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1295826309
Hi, can you enable GitHub Actions on yours repo?
--
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 #38433: [SPARK-40943][SQL] Make the MSCK keyword optional in REPAIR TABLE commands
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38433:
URL: https://github.com/apache/spark/pull/38433#issuecomment-1301158936
Thank you so much, @ben-zhang .
--
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