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 2020/02/03 21:17:53 UTC
[GitHub] [spark] imback82 opened a new pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
imback82 opened a new pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444
<!--
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.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
The current ALTER COLUMN syntax allows to change multiple properties at a time:
```
ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
(TYPE dataType)?
(COMMENT comment=STRING)?
colPosition?
```
The SQL standard (section 11.12) only allows changing one property at a time. This is also true on other recent SQL systems like [snowflake](https://docs.snowflake.net/manuals/sql-reference/sql/alter-table-column.html) and [redshift](https://docs.aws.amazon.com/redshift/latest/dg/r_ALTER_TABLE.html). (credit to @cloud-fan)
This PR proposes to change ALTER COLUMN to follow SQL standard, thus allows altering only one column property at a time.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
To follow SQL standard (and other recent SQL systems) behavior.
### Does this PR introduce any user-facing change?
<!--
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 no, write 'No'.
-->
Yes, now the user can update the column properties only one at a time.
For example,
```
ALTER TABLE table1 ALTER COLUMN a.b.c TYPE bigint COMMENT 'new comment'
```
should be broken into
```
ALTER TABLE table1 ALTER COLUMN a.b.c TYPE bigint
ALTER TABLE table1 ALTER COLUMN a.b.c COMMENT 'new comment'
```
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Updated existing 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582232032
**[Test build #117887 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117887/testReport)** for PR 27444 at commit [`192d805`](https://github.com/apache/spark/commit/192d805b85522df6b95ca65fcf0d699fe6f9d1f9).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582730003
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117956/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581625345
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
maropu commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582241638
Ah, I see. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582674762
**[Test build #117956 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117956/testReport)** for PR 27444 at commit [`93de5f2`](https://github.com/apache/spark/commit/93de5f27906a2292d1749f684184877ed207a9c2).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581738626
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22565/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375071874
##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -158,12 +158,9 @@ statement
SET TBLPROPERTIES tablePropertyList #setTableProperties
| ALTER (TABLE | VIEW) multipartIdentifier
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList #unsetTableProperties
- | ALTER TABLE table=multipartIdentifier
+ |ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
- (TYPE dataType)? commentSpec? colPosition? #alterTableColumn
- | ALTER TABLE table=multipartIdentifier
Review comment:
Will update 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375460831
##########
File path: sql/core/src/test/resources/sql-tests/results/change-column.sql.out
##########
@@ -27,11 +27,11 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException
-Operation not allowed: ALTER TABLE table CHANGE COLUMN requires a TYPE or a COMMENT or a FIRST/AFTER(line 1, pos 0)
Review comment:
OK. I will update it.
FYI, in Postgres:
```SQL
postgres=# alter table t alter column i;
2020-02-05 14:26:22.056 EST [86874] ERROR: syntax error at or near ";" at character 29
2020-02-05 14:26:22.056 EST [86874] STATEMENT: alter table t alter column i;
ERROR: syntax error at or near ";"
LINE 1: alter table t alter column i;
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581701999
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581625353
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22548/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581624743
**[Test build #117785 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117785/testReport)** for PR 27444 at commit [`720f8fe`](https://github.com/apache/spark/commit/720f8fede4f906dd8a8abf9f45eb9259732b40c7).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581753883
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375066771
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -92,17 +88,27 @@ class ResolveSessionCatalog(
val builder = new MetadataBuilder
// Add comment to metadata
a.comment.map(c => builder.putString("comment", c))
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse {
+ v1Table.schema.findNestedField(Seq(colName), resolver = conf.resolver)
+ .map(_._2.dataType)
+ .getOrElse {
+ throw new AnalysisException(
+ s"ALTER COLUMN cannot find column ${quote(colName)} in v1 table. " +
+ s"Available: ${v1Table.schema.fieldNames.mkString(", ")}")
+ }
+ }
// Add Hive type string to metadata.
- val cleanedDataType = HiveStringType.replaceCharType(a.dataType.get)
- if (a.dataType.get != cleanedDataType) {
+ val cleanedDataType = HiveStringType.replaceCharType(dataType)
+ if (dataType != cleanedDataType) {
builder.putString(HIVE_TYPE_STRING, a.dataType.get.catalogString)
}
Review comment:
`builder.putString(HIVE_TYPE_STRING, dataType)`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581660447
**[Test build #117786 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117786/testReport)** for PR 27444 at commit [`f4611e7`](https://github.com/apache/spark/commit/f4611e7ecb2ff6a8424e72b5cb1593d818fda6e0).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581738394
**[Test build #117802 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117802/testReport)** for PR 27444 at commit [`1965e5d`](https://github.com/apache/spark/commit/1965e5d53a9d9500ecde96c46a0948b97267041c).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582287934
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117887/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581653366
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117785/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581653359
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375024492
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -89,20 +85,23 @@ class ResolveSessionCatalog(
throw new AnalysisException("" +
"ALTER COLUMN ... FIRST | ALTER is only supported with v2 tables.")
}
+
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse(v1Table.schema(colName).dataType)
Review comment:
Sure.
I noticed that we are not considering case sensitivities in v2 tables either: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala#L432. I can do a follow up PR for this if needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374480585
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -89,20 +85,23 @@ class ResolveSessionCatalog(
throw new AnalysisException("" +
"ALTER COLUMN ... FIRST | ALTER is only supported with v2 tables.")
}
+
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse(v1Table.schema(colName).dataType)
Review comment:
shall we consider case sensitivity?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582287647
**[Test build #117887 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117887/testReport)** for PR 27444 at commit [`192d805`](https://github.com/apache/spark/commit/192d805b85522df6b95ca65fcf0d699fe6f9d1f9).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-583550907
thanks, merging to master/3.0!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581738622
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582729999
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581660519
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375024676
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
##########
@@ -1012,27 +1012,28 @@ class PlanResolutionSuite extends AnalysisTest {
Seq("v1Table" -> true, "v2Table" -> false, "testcat.tab" -> false).foreach {
case (tblName, useV1Command) =>
val sql1 = s"ALTER TABLE $tblName ALTER COLUMN i TYPE bigint"
- val sql2 = s"ALTER TABLE $tblName ALTER COLUMN i TYPE bigint COMMENT 'new comment'"
- val sql3 = s"ALTER TABLE $tblName ALTER COLUMN i COMMENT 'new comment'"
+ val sql2 = s"ALTER TABLE $tblName ALTER COLUMN i COMMENT 'new comment'"
val parsed1 = parseAndResolve(sql1)
val parsed2 = parseAndResolve(sql2)
val tableIdent = TableIdentifier(tblName, None)
if (useV1Command) {
+ val oldColumn = StructField("i", IntegerType)
val newColumn = StructField("i", LongType)
val expected1 = AlterTableChangeColumnCommand(
tableIdent, "i", newColumn)
val expected2 = AlterTableChangeColumnCommand(
- tableIdent, "i", newColumn.withComment("new comment"))
+ tableIdent, "i", oldColumn.withComment("new comment"))
comparePlans(parsed1, expected1)
comparePlans(parsed2, expected2)
- val e1 = intercept[AnalysisException] {
+ val sql3 = s"ALTER TABLE $tblName ALTER COLUMN j COMMENT 'new comment'"
+ val e1 = intercept[IllegalArgumentException] {
parseAndResolve(sql3)
}
- assert(e1.getMessage.contains("ALTER COLUMN with v1 tables must specify new data type"))
+ assert(e1.getMessage.contains("j does not exist. Available: i, s"))
Review comment:
This is actually a v1 table path. For v2 tables, column names are resolved in `CheckAnalysis`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581753491
**[Test build #117795 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117795/testReport)** for PR 27444 at commit [`ba23c68`](https://github.com/apache/spark/commit/ba23c68f61b038a5c15ed0354fd49dce0df875d9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581724601
BTW I'm not against having multiple column changes within one command, but we need more discussion to decide what syntax to use since it's non-standard.
One candidate is the snowflake syntax
```
ALTER TABLE <name> { ALTER | MODIFY } [ ( ]
[ COLUMN ] <col1_name> DROP DEFAULT
, [ COLUMN ] <col1_name> SET DEFAULT <seq_name>.NEXTVAL
, [ COLUMN ] <col1_name> { [ SET ] NOT NULL | DROP NOT NULL }
, [ COLUMN ] <col1_name> [ [ SET DATA ] TYPE ] <type>
, [ COLUMN ] <col1_name> COMMENT '<string>'
[ , [ COLUMN ] <col2_name> ... ]
```
We don't need to rush it for 3.0. We can be conservative in 3.0 to only support the SQL standard syntax, and think about how to extend it later.
cc @viirya @maropu @brkyvz @rdblue
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375023741
##########
File path: sql/core/src/test/resources/sql-tests/inputs/change-column.sql
##########
@@ -15,18 +15,19 @@ ALTER TABLE test_change CHANGE a TYPE STRING;
DESC test_change;
-- Change column position (not supported yet)
-ALTER TABLE test_change CHANGE a TYPE INT AFTER b;
-ALTER TABLE test_change CHANGE b TYPE STRING FIRST;
+ALTER TABLE test_change CHANGE INT AFTER b;
Review comment:
oops, thanks for catching 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581701136
Thanks @dongjoon-hyun for the heads up.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374361369
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2940,30 +2940,61 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Parse a [[AlterTableAlterColumnStatement]] command.
+ * Parse a [[AlterTableAlterColumnStatement]] command to change column type
*
* For example:
* {{{
* ALTER TABLE table1 ALTER COLUMN a.b.c TYPE bigint
- * ALTER TABLE table1 ALTER COLUMN a.b.c TYPE bigint COMMENT 'new comment'
+ * }}}
+ */
+ override def visitAlterTableColumnType(
+ ctx: AlterTableColumnTypeContext): LogicalPlan = withOrigin(ctx) {
+ AlterTableAlterColumnStatement(
+ visitMultipartIdentifier(ctx.table),
+ typedVisit[Seq[String]](ctx.column),
+ dataType = Some(typedVisit[DataType](ctx.dataType)),
+ nullable = None,
+ comment = None,
+ position = None)
+ }
+
+ /**
+ * Parse a [[AlterTableAlterColumnStatement]] command to change column comment
+ *
+ * For example:
+ * {{{
* ALTER TABLE table1 ALTER COLUMN a.b.c COMMENT 'new comment'
* }}}
*/
- override def visitAlterTableColumn(
- ctx: AlterTableColumnContext): LogicalPlan = withOrigin(ctx) {
- val verb = if (ctx.CHANGE != null) "CHANGE" else "ALTER"
- if (ctx.dataType == null && ctx.commentSpec() == null && ctx.colPosition == null) {
- operationNotAllowed(
- s"ALTER TABLE table $verb COLUMN requires a TYPE or a COMMENT or a FIRST/AFTER", ctx)
- }
+ override def visitAlterTableColumnComment(
+ ctx: AlterTableColumnCommentContext): LogicalPlan = withOrigin(ctx) {
+ AlterTableAlterColumnStatement(
+ visitMultipartIdentifier(ctx.table),
+ typedVisit[Seq[String]](ctx.column),
+ dataType = None,
+ nullable = None,
+ comment = Some(visitCommentSpec(ctx.commentSpec())),
+ position = None)
+ }
+ /**
+ * Parse a [[AlterTableAlterColumnStatement]] command to change column position
+ *
+ * For example:
+ * {{{
+ * ALTER TABLE table1 ALTER COLUMN a.b.c FIRST
+ * ALTER TABLE table1 ALTER COLUMN a.b.c AFTER x
+ * }}}
+ */
+ override def visitAlterTableColumnPosition(
+ ctx: AlterTableColumnPositionContext): LogicalPlan = withOrigin(ctx) {
AlterTableAlterColumnStatement(
visitMultipartIdentifier(ctx.table),
typedVisit[Seq[String]](ctx.column),
- dataType = Option(ctx.dataType).map(typedVisit[DataType]),
+ dataType = None,
nullable = None,
- comment = Option(ctx.commentSpec()).map(visitCommentSpec),
- position = Option(ctx.colPosition).map(typedVisit[ColumnPosition]))
+ comment = None,
+ position = Some(typedVisit[ColumnPosition](ctx.colPosition)))
}
Review comment:
I could also update `AlterTAbleAlterColumnStatement` to be like:
```Scala
case class AlterTableAlterColumnStatement(
tableName: Seq[String],
column: Seq[String],
dataType: Option[DataType],
nullable: Option[Boolean],
comment: Option[String],
position: Option[ColumnPosition],
isHive: Boolean) extends ParsedStatement {
require(
isHive || Seq(dataType, nullable, comment, position).filter(_.nonEmpty).length == 1,
"Only one property can be changed at a time.")
}
```
to be safe, but I didn't add it because of `isHive` hack.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375024492
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -89,20 +85,23 @@ class ResolveSessionCatalog(
throw new AnalysisException("" +
"ALTER COLUMN ... FIRST | ALTER is only supported with v2 tables.")
}
+
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse(v1Table.schema(colName).dataType)
Review comment:
Sure.
I noticed that we are not considering case sensitivities in v2 tables either: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala#L432. I can do a follow up PR for this if needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581664555
Could you update `change-column.sql` and `InMemoryCatalogedDDLSuite` accordingly?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375059871
##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -158,12 +158,9 @@ statement
SET TBLPROPERTIES tablePropertyList #setTableProperties
| ALTER (TABLE | VIEW) multipartIdentifier
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList #unsetTableProperties
- | ALTER TABLE table=multipartIdentifier
+ |ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
- (TYPE dataType)? commentSpec? colPosition? #alterTableColumn
- | ALTER TABLE table=multipartIdentifier
Review comment:
a note to other reviewers: the ALTER COLUMN syntax being changed here is newly added in 3.0.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581788649
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117802/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375034374
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -89,20 +85,23 @@ class ResolveSessionCatalog(
throw new AnalysisException("" +
"ALTER COLUMN ... FIRST | ALTER is only supported with v2 tables.")
}
+
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse(v1Table.schema(colName).dataType)
Review comment:
Sure, will add.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581660519
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581738394
**[Test build #117802 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117802/testReport)** for PR 27444 at commit [`1965e5d`](https://github.com/apache/spark/commit/1965e5d53a9d9500ecde96c46a0948b97267041c).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374346923
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala
##########
@@ -646,19 +646,6 @@ class DDLParserSuite extends AnalysisTest {
Some(first())))
}
- test("alter table: update column type, comment and position") {
- comparePlans(
- parsePlan("ALTER TABLE table_name CHANGE COLUMN a.b.c " +
- "TYPE bigint COMMENT 'new comment' AFTER d"),
Review comment:
Note that this now matches HIVE style alter column:
```
ALTER TABLE table [PARTITION partition_spec]
CHANGE [COLUMN] column_old_name column_new_name column_dataType
[COMMENT column_comment]
[FIRST | AFTER column_name];
```
And reports `ParseException: Renaming column is not supported in Hive-style ALTER COLUMN, please run RENAME COLUMN instead.` because `TYPE` is parsed as a new name.
So I changed `CHANGE COLUMN` to `ALTER COLUMN` for testing invalid parsing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582232032
**[Test build #117887 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117887/testReport)** for PR 27444 at commit [`192d805`](https://github.com/apache/spark/commit/192d805b85522df6b95ca65fcf0d699fe6f9d1f9).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582729999
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581702001
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22558/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581701716
**[Test build #117795 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117795/testReport)** for PR 27444 at commit [`ba23c68`](https://github.com/apache/spark/commit/ba23c68f61b038a5c15ed0354fd49dce0df875d9).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375024492
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -89,20 +85,23 @@ class ResolveSessionCatalog(
throw new AnalysisException("" +
"ALTER COLUMN ... FIRST | ALTER is only supported with v2 tables.")
}
+
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse(v1Table.schema(colName).dataType)
Review comment:
Sure. I noticed that we are not considering case sensitivities in v2 tables either: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala#L432
I can do a follow up PR if needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582675136
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374361369
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2940,30 +2940,61 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Parse a [[AlterTableAlterColumnStatement]] command.
+ * Parse a [[AlterTableAlterColumnStatement]] command to change column type
*
* For example:
* {{{
* ALTER TABLE table1 ALTER COLUMN a.b.c TYPE bigint
- * ALTER TABLE table1 ALTER COLUMN a.b.c TYPE bigint COMMENT 'new comment'
+ * }}}
+ */
+ override def visitAlterTableColumnType(
+ ctx: AlterTableColumnTypeContext): LogicalPlan = withOrigin(ctx) {
+ AlterTableAlterColumnStatement(
+ visitMultipartIdentifier(ctx.table),
+ typedVisit[Seq[String]](ctx.column),
+ dataType = Some(typedVisit[DataType](ctx.dataType)),
+ nullable = None,
+ comment = None,
+ position = None)
+ }
+
+ /**
+ * Parse a [[AlterTableAlterColumnStatement]] command to change column comment
+ *
+ * For example:
+ * {{{
* ALTER TABLE table1 ALTER COLUMN a.b.c COMMENT 'new comment'
* }}}
*/
- override def visitAlterTableColumn(
- ctx: AlterTableColumnContext): LogicalPlan = withOrigin(ctx) {
- val verb = if (ctx.CHANGE != null) "CHANGE" else "ALTER"
- if (ctx.dataType == null && ctx.commentSpec() == null && ctx.colPosition == null) {
- operationNotAllowed(
- s"ALTER TABLE table $verb COLUMN requires a TYPE or a COMMENT or a FIRST/AFTER", ctx)
- }
+ override def visitAlterTableColumnComment(
+ ctx: AlterTableColumnCommentContext): LogicalPlan = withOrigin(ctx) {
+ AlterTableAlterColumnStatement(
+ visitMultipartIdentifier(ctx.table),
+ typedVisit[Seq[String]](ctx.column),
+ dataType = None,
+ nullable = None,
+ comment = Some(visitCommentSpec(ctx.commentSpec())),
+ position = None)
+ }
+ /**
+ * Parse a [[AlterTableAlterColumnStatement]] command to change column position
+ *
+ * For example:
+ * {{{
+ * ALTER TABLE table1 ALTER COLUMN a.b.c FIRST
+ * ALTER TABLE table1 ALTER COLUMN a.b.c AFTER x
+ * }}}
+ */
+ override def visitAlterTableColumnPosition(
+ ctx: AlterTableColumnPositionContext): LogicalPlan = withOrigin(ctx) {
AlterTableAlterColumnStatement(
visitMultipartIdentifier(ctx.table),
typedVisit[Seq[String]](ctx.column),
- dataType = Option(ctx.dataType).map(typedVisit[DataType]),
+ dataType = None,
nullable = None,
- comment = Option(ctx.commentSpec()).map(visitCommentSpec),
- position = Option(ctx.colPosition).map(typedVisit[ColumnPosition]))
+ comment = None,
+ position = Some(typedVisit[ColumnPosition](ctx.colPosition)))
}
Review comment:
I could also update `AlterTAbleAlterColumnStatement` to be like:
```Scala
case class AlterTableAlterColumnStatement(
tableName: Seq[String],
column: Seq[String],
dataType: Option[DataType],
nullable: Option[Boolean],
comment: Option[String],
position: Option[ColumnPosition],
isHive: Boolean) extends ParsedStatement {
require(
isHive || Seq(dataType, nullable, comment, position).filter(_.nonEmpty).length == 1,
"Only one property can be changed at a time.")
}
```
to be safe, but I didn't add it because of `isHive` hack.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581753883
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581628569
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22549/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581753887
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117795/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582240776
@maropu before 3.0 we only have hive style ALTER COLUMN syntax (which can change multiple properties at one time), the syntax touched here is newly added in 3.0. That's why I think it's better to make it follow SQL standard first, and have more discussions about how to extend 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581628569
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22549/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581653366
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117785/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581788649
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117802/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581628060
**[Test build #117786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117786/testReport)** for PR 27444 at commit [`f4611e7`](https://github.com/apache/spark/commit/f4611e7ecb2ff6a8424e72b5cb1593d818fda6e0).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375070537
##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -158,12 +158,9 @@ statement
SET TBLPROPERTIES tablePropertyList #setTableProperties
| ALTER (TABLE | VIEW) multipartIdentifier
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList #unsetTableProperties
- | ALTER TABLE table=multipartIdentifier
+ |ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
- (TYPE dataType)? commentSpec? colPosition? #alterTableColumn
- | ALTER TABLE table=multipartIdentifier
- ALTER COLUMN? column=multipartIdentifier
- setOrDrop=(SET | DROP) NOT NULL #alterColumnNullability
Review comment:
I don't have a strong preference as long as they are consistent. Since the hive style alter column use `CHANGE` and we support that syntax for a long time, I'm OK to support `CHANGE` as well in the new syntax.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581660525
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117786/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581738626
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22565/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582232311
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581701716
**[Test build #117795 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117795/testReport)** for PR 27444 at commit [`ba23c68`](https://github.com/apache/spark/commit/ba23c68f61b038a5c15ed0354fd49dce0df875d9).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582287926
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582287926
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375060713
##########
File path: sql/core/src/test/resources/sql-tests/results/change-column.sql.out
##########
@@ -27,11 +27,11 @@ struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException
-Operation not allowed: ALTER TABLE table CHANGE COLUMN requires a TYPE or a COMMENT or a FIRST/AFTER(line 1, pos 0)
Review comment:
The previous error message is better. Maybe we should make `alterColumnAction` optional, and throw error with nice message if it's not specified.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581788639
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581701999
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581738622
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582674762
**[Test build #117956 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117956/testReport)** for PR 27444 at commit [`93de5f2`](https://github.com/apache/spark/commit/93de5f27906a2292d1749f684184877ed207a9c2).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375024676
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
##########
@@ -1012,27 +1012,28 @@ class PlanResolutionSuite extends AnalysisTest {
Seq("v1Table" -> true, "v2Table" -> false, "testcat.tab" -> false).foreach {
case (tblName, useV1Command) =>
val sql1 = s"ALTER TABLE $tblName ALTER COLUMN i TYPE bigint"
- val sql2 = s"ALTER TABLE $tblName ALTER COLUMN i TYPE bigint COMMENT 'new comment'"
- val sql3 = s"ALTER TABLE $tblName ALTER COLUMN i COMMENT 'new comment'"
+ val sql2 = s"ALTER TABLE $tblName ALTER COLUMN i COMMENT 'new comment'"
val parsed1 = parseAndResolve(sql1)
val parsed2 = parseAndResolve(sql2)
val tableIdent = TableIdentifier(tblName, None)
if (useV1Command) {
+ val oldColumn = StructField("i", IntegerType)
val newColumn = StructField("i", LongType)
val expected1 = AlterTableChangeColumnCommand(
tableIdent, "i", newColumn)
val expected2 = AlterTableChangeColumnCommand(
- tableIdent, "i", newColumn.withComment("new comment"))
+ tableIdent, "i", oldColumn.withComment("new comment"))
comparePlans(parsed1, expected1)
comparePlans(parsed2, expected2)
- val e1 = intercept[AnalysisException] {
+ val sql3 = s"ALTER TABLE $tblName ALTER COLUMN j COMMENT 'new comment'"
+ val e1 = intercept[IllegalArgumentException] {
parseAndResolve(sql3)
}
- assert(e1.getMessage.contains("ALTER COLUMN with v1 tables must specify new data type"))
+ assert(e1.getMessage.contains("j does not exist. Available: i, s"))
Review comment:
Actually, this is a v1 table path. For v2 tables, column names are resolved in `CheckAnalysis`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581628060
**[Test build #117786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117786/testReport)** for PR 27444 at commit [`f4611e7`](https://github.com/apache/spark/commit/f4611e7ecb2ff6a8424e72b5cb1593d818fda6e0).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374477802
##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -158,12 +158,9 @@ statement
SET TBLPROPERTIES tablePropertyList #setTableProperties
| ALTER (TABLE | VIEW) multipartIdentifier
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList #unsetTableProperties
- | ALTER TABLE table=multipartIdentifier
- (ALTER | CHANGE) COLUMN? column=multipartIdentifier
- (TYPE dataType)? commentSpec? colPosition? #alterTableColumn
- | ALTER TABLE table=multipartIdentifier
- ALTER COLUMN? column=multipartIdentifier
- setOrDrop=(SET | DROP) NOT NULL #alterColumnNullability
+ |ALTER TABLE table=multipartIdentifier
+ (ALTER | CHANGE) COLUMN? column=multipartIdentifier
Review comment:
nit: wrong indentation?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581788639
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581660525
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117786/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374478446
##########
File path: sql/core/src/test/resources/sql-tests/inputs/change-column.sql
##########
@@ -15,18 +15,19 @@ ALTER TABLE test_change CHANGE a TYPE STRING;
DESC test_change;
-- Change column position (not supported yet)
-ALTER TABLE test_change CHANGE a TYPE INT AFTER b;
-ALTER TABLE test_change CHANGE b TYPE STRING FIRST;
+ALTER TABLE test_change CHANGE INT AFTER b;
Review comment:
we still need the column name...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581624743
**[Test build #117785 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117785/testReport)** for PR 27444 at commit [`720f8fe`](https://github.com/apache/spark/commit/720f8fede4f906dd8a8abf9f45eb9259732b40c7).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582802412
@rdblue @brkyvz any concerns? I'd like to avoid releasing an extended ALTER COLUMN syntax without sufficient discussion to make sure people are good with the extension.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375071833
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -92,17 +88,27 @@ class ResolveSessionCatalog(
val builder = new MetadataBuilder
// Add comment to metadata
a.comment.map(c => builder.putString("comment", c))
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse {
+ v1Table.schema.findNestedField(Seq(colName), resolver = conf.resolver)
+ .map(_._2.dataType)
+ .getOrElse {
+ throw new AnalysisException(
+ s"ALTER COLUMN cannot find column ${quote(colName)} in v1 table. " +
+ s"Available: ${v1Table.schema.fieldNames.mkString(", ")}")
+ }
+ }
// Add Hive type string to metadata.
- val cleanedDataType = HiveStringType.replaceCharType(a.dataType.get)
- if (a.dataType.get != cleanedDataType) {
+ val cleanedDataType = HiveStringType.replaceCharType(dataType)
+ if (dataType != cleanedDataType) {
builder.putString(HIVE_TYPE_STRING, a.dataType.get.catalogString)
}
Review comment:
good catch. it should be `builder.putString(HIVE_TYPE_STRING, dataType.catalogString)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374455848
##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -160,7 +160,13 @@ statement
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList #unsetTableProperties
| ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
- (TYPE dataType)? commentSpec? colPosition? #alterTableColumn
+ TYPE dataType #alterTableColumnType
+ | ALTER TABLE table=multipartIdentifier
Review comment:
I'd like to make the syntax rule cleaner
```
ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
alterColumnAction
alterColumnAction
: TYPE dataType
| commentSpec
| colPosition
| setOrDrop=(SET | DROP) NOT NULL
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581628555
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582287934
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117887/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581653359
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375064422
##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -158,12 +158,9 @@ statement
SET TBLPROPERTIES tablePropertyList #setTableProperties
| ALTER (TABLE | VIEW) multipartIdentifier
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList #unsetTableProperties
- | ALTER TABLE table=multipartIdentifier
+ |ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
- (TYPE dataType)? commentSpec? colPosition? #alterTableColumn
- | ALTER TABLE table=multipartIdentifier
Review comment:
We could clarify this at the PR description.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
maropu commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582238585
We don't need to keep the legacy(existing) behaviour? I think the current one still looks useful for users. As @cloud-fan said above, a new syntax for multiple column changes looks fine to me. But, we need more discussion for that, so I think its worth keeping the current one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581653224
**[Test build #117785 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117785/testReport)** for PR 27444 at commit [`720f8fe`](https://github.com/apache/spark/commit/720f8fede4f906dd8a8abf9f45eb9259732b40c7).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581625353
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22548/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582675136
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582729543
**[Test build #117956 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117956/testReport)** for PR 27444 at commit [`93de5f2`](https://github.com/apache/spark/commit/93de5f27906a2292d1749f684184877ed207a9c2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375065432
##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -158,12 +158,9 @@ statement
SET TBLPROPERTIES tablePropertyList #setTableProperties
| ALTER (TABLE | VIEW) multipartIdentifier
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList #unsetTableProperties
- | ALTER TABLE table=multipartIdentifier
+ |ALTER TABLE table=multipartIdentifier
(ALTER | CHANGE) COLUMN? column=multipartIdentifier
- (TYPE dataType)? commentSpec? colPosition? #alterTableColumn
- | ALTER TABLE table=multipartIdentifier
- ALTER COLUMN? column=multipartIdentifier
- setOrDrop=(SET | DROP) NOT NULL #alterColumnNullability
Review comment:
`alterColumnNullability` accepts only `ALTER` not `CHANGE` previously. After this, `CHANGE COLUMN` also works for `setOrDrop`. Is this on purpose?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582232315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22649/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27444: [SPARK-30614][SQL] The
native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581788334
**[Test build #117802 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117802/testReport)** for PR 27444 at commit [`1965e5d`](https://github.com/apache/spark/commit/1965e5d53a9d9500ecde96c46a0948b97267041c).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r375051390
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -89,20 +85,23 @@ class ResolveSessionCatalog(
throw new AnalysisException("" +
"ALTER COLUMN ... FIRST | ALTER is only supported with v2 tables.")
}
+
+ val colName = a.column(0)
+ val dataType = a.dataType.getOrElse(v1Table.schema(colName).dataType)
Review comment:
(btw, good catch!, it was a bug not considering case sensitivity)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582232315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22649/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582730003
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117956/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581702001
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22558/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582232311
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582675149
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22720/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374346923
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala
##########
@@ -646,19 +646,6 @@ class DDLParserSuite extends AnalysisTest {
Some(first())))
}
- test("alter table: update column type, comment and position") {
- comparePlans(
- parsePlan("ALTER TABLE table_name CHANGE COLUMN a.b.c " +
- "TYPE bigint COMMENT 'new comment' AFTER d"),
Review comment:
This now matches HIVE style alter column:
```
ALTER TABLE table [PARTITION partition_spec]
CHANGE [COLUMN] column_old_name column_new_name column_dataType
[COMMENT column_comment]
[FIRST | AFTER column_name];
```
And reports `ParseException: Renaming column is not supported in Hive-style ALTER COLUMN, please run RENAME COLUMN instead.` because `TYPE` is parsed as a new name.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581628555
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#discussion_r374479332
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
##########
@@ -1012,27 +1012,28 @@ class PlanResolutionSuite extends AnalysisTest {
Seq("v1Table" -> true, "v2Table" -> false, "testcat.tab" -> false).foreach {
case (tblName, useV1Command) =>
val sql1 = s"ALTER TABLE $tblName ALTER COLUMN i TYPE bigint"
- val sql2 = s"ALTER TABLE $tblName ALTER COLUMN i TYPE bigint COMMENT 'new comment'"
- val sql3 = s"ALTER TABLE $tblName ALTER COLUMN i COMMENT 'new comment'"
+ val sql2 = s"ALTER TABLE $tblName ALTER COLUMN i COMMENT 'new comment'"
val parsed1 = parseAndResolve(sql1)
val parsed2 = parseAndResolve(sql2)
val tableIdent = TableIdentifier(tblName, None)
if (useV1Command) {
+ val oldColumn = StructField("i", IntegerType)
val newColumn = StructField("i", LongType)
val expected1 = AlterTableChangeColumnCommand(
tableIdent, "i", newColumn)
val expected2 = AlterTableChangeColumnCommand(
- tableIdent, "i", newColumn.withComment("new comment"))
+ tableIdent, "i", oldColumn.withComment("new comment"))
comparePlans(parsed1, expected1)
comparePlans(parsed2, expected2)
- val e1 = intercept[AnalysisException] {
+ val sql3 = s"ALTER TABLE $tblName ALTER COLUMN j COMMENT 'new comment'"
+ val e1 = intercept[IllegalArgumentException] {
parseAndResolve(sql3)
}
- assert(e1.getMessage.contains("ALTER COLUMN with v1 tables must specify new data type"))
+ assert(e1.getMessage.contains("j does not exist. Available: i, s"))
Review comment:
shall we test it for v2 table as well?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-582675149
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22720/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #27444:
[SPARK-30614][SQL] The native ALTER COLUMN syntax should change one
property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581753887
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117795/
Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27444: [SPARK-30614][SQL]
The native ALTER COLUMN syntax should change one property at a time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27444: [SPARK-30614][SQL] The native ALTER COLUMN syntax should change one property at a time
URL: https://github.com/apache/spark/pull/27444#issuecomment-581625345
Merged build finished. Test PASSed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org