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/10/14 18:37:56 UTC
[GitHub] [spark] huaxingao opened a new pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
huaxingao opened a new pull request #30041:
URL: https://github.com/apache/spark/pull/30041
### What changes were proposed in this pull request?
I currently have unquoted column names in alter table, e.g. ```ALTER TABLE "test"."alt_table" DROP COLUMN c1```
should change to quoted column name ```ALTER TABLE "test"."alt_table" DROP COLUMN "c1"```
### Why are the changes needed?
We should always use quoted identifiers in JDBC SQLs, e.g. ```CREATE TABLE "test"."abc" ("col" INTEGER ) ``` or ```INSERT INTO "test"."abc" ("col") VALUES (?)```. Using unquoted column name in alterTable causes problems, for example:
```
sql("CREATE TABLE h2.test.alt_table (c1 INTEGER, c2 INTEGER) USING _")
sql("ALTER TABLE h2.test.alt_table DROP COLUMN c1")
org.apache.spark.sql.AnalysisException: Failed table altering: test.alt_table;
......
Caused by: org.h2.jdbc.JdbcSQLException: Column "C1" not found; SQL statement:
ALTER TABLE "test"."alt_table" DROP COLUMN c1 [42122-195]
```
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708601572
**[Test build #129744 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129744/testReport)** for PR 30041 at commit [`cb0de33`](https://github.com/apache/spark/commit/cb0de33e82fcc4e7d6a9e58ac1baf02ab45bef69).
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708946838
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/129808/
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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708601572
**[Test build #129744 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129744/testReport)** for PR 30041 at commit [`cb0de33`](https://github.com/apache/spark/commit/cb0de33e82fcc4e7d6a9e58ac1baf02ab45bef69).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708606026
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30041:
URL: https://github.com/apache/spark/pull/30041#discussion_r505202940
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -272,10 +274,12 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
test("alter table ... update column nullability") {
withTable("h2.test.alt_table") {
- sql("CREATE TABLE h2.test.alt_table (ID INTEGER NOT NULL) USING _")
+ sql("CREATE TABLE h2.test.alt_table (ID INTEGER NOT NULL, deptno INTEGER NOT NULL) USING _")
Review comment:
Without fix (with unquoted column name), this throws the following Exception:
```
Failed table altering: test.alt_table;
org.apache.spark.sql.AnalysisException: Failed table altering: test.alt_table;
at org.apache.spark.sql.jdbc.JdbcDialect.classifyException(JdbcDialects.scala:268)
Caused by: org.h2.jdbc.JdbcSQLException: Column "DEPTNO" not found; SQL statement:
ALTER TABLE "test"."alt_table" ALTER COLUMN deptno SET NULL [42122-195]
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708606037
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34350/
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30041:
URL: https://github.com/apache/spark/pull/30041#discussion_r505669033
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -199,8 +199,8 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
test("alter table ... rename column") {
withTable("h2.test.alt_table") {
- sql("CREATE TABLE h2.test.alt_table (ID INTEGER, C0 INTEGER) USING _")
- sql("ALTER TABLE h2.test.alt_table RENAME COLUMN ID TO C")
+ sql("CREATE TABLE h2.test.alt_table (id INTEGER, C0 INTEGER) USING _")
+ sql("ALTER TABLE h2.test.alt_table RENAME COLUMN id TO C")
Review comment:
Yes, upper case `ID` still works here. I will add tests for case insensitive column resolution.
Thanks for reviewing and merging my PR!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708933402
I actually get a little confused now: do I also need to take consideration of `spark.sql.caseSensitive`?
I will take a look at this tomorrow.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30041:
URL: https://github.com/apache/spark/pull/30041
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708606026
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708970518
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708936553
> I actually get a little confused now: do I also need to take consideration of spark.sql.caseSensitive?
I think there was a discussion a long time ago. For table/column lookup that happens inside custom catalogs, Spark can't control it. `spark.sql.caseSensitive` only controls lookup behavior inside spark.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708765402
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708762527
**[Test build #129744 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129744/testReport)** for PR 30041 at commit [`cb0de33`](https://github.com/apache/spark/commit/cb0de33e82fcc4e7d6a9e58ac1baf02ab45bef69).
* 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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708765402
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30041:
URL: https://github.com/apache/spark/pull/30041#discussion_r505202754
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -223,8 +223,9 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
test("alter table ... drop column") {
withTable("h2.test.alt_table") {
- sql("CREATE TABLE h2.test.alt_table (C1 INTEGER, C2 INTEGER) USING _")
+ sql("CREATE TABLE h2.test.alt_table (C1 INTEGER, C2 INTEGER, c3 INTEGER) USING _")
sql("ALTER TABLE h2.test.alt_table DROP COLUMN C1")
+ sql("ALTER TABLE h2.test.alt_table DROP COLUMN c3")
Review comment:
Without fix (with unquoted column name), this throws the following Exception:
```
Failed table altering: test.alt_table;
org.apache.spark.sql.AnalysisException: Failed table altering: test.alt_table;
at org.apache.spark.sql.jdbc.JdbcDialect.classifyException(JdbcDialects.scala:268)
Caused by: org.h2.jdbc.JdbcSQLException: Column "C3" not found; SQL statement:
ALTER TABLE "test"."alt_table" DROP COLUMN c3 [42122-195]
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -245,10 +246,11 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
test("alter table ... update column type") {
withTable("h2.test.alt_table") {
- sql("CREATE TABLE h2.test.alt_table (ID INTEGER) USING _")
+ sql("CREATE TABLE h2.test.alt_table (ID INTEGER, deptno INTEGER) USING _")
sql("ALTER TABLE h2.test.alt_table ALTER COLUMN id TYPE DOUBLE")
+ sql("ALTER TABLE h2.test.alt_table ALTER COLUMN deptno TYPE DOUBLE")
Review comment:
Without fix (with unquoted column name), this throws the following Exception:
```
Failed table altering: test.alt_table;
org.apache.spark.sql.AnalysisException: Failed table altering: test.alt_table;
at org.apache.spark.sql.jdbc.JdbcDialect.classifyException(JdbcDialects.scala:268)
Caused by: org.h2.jdbc.JdbcSQLException: Column "DEPTNO" not found; SQL statement:
ALTER TABLE "test"."alt_table" ALTER COLUMN deptno DOUBLE PRECISION [42122-195]
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708946586
**[Test build #129808 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129808/testReport)** for PR 30041 at commit [`7e8098c`](https://github.com/apache/spark/commit/7e8098cebdfae077205253028e48d7b5bf9a8c07).
* 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708946825
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708929217
**[Test build #129808 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129808/testReport)** for PR 30041 at commit [`7e8098c`](https://github.com/apache/spark/commit/7e8098cebdfae077205253028e48d7b5bf9a8c07).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-709406951
thanks, merging to master!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708958098
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34413/
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708946825
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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708970518
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708877231
@cloud-fan @MaxGekk Could you please take a look? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30041:
URL: https://github.com/apache/spark/pull/30041#discussion_r505642672
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -199,8 +199,8 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
test("alter table ... rename column") {
withTable("h2.test.alt_table") {
- sql("CREATE TABLE h2.test.alt_table (ID INTEGER, C0 INTEGER) USING _")
- sql("ALTER TABLE h2.test.alt_table RENAME COLUMN ID TO C")
+ sql("CREATE TABLE h2.test.alt_table (id INTEGER, C0 INTEGER) USING _")
+ sql("ALTER TABLE h2.test.alt_table RENAME COLUMN id TO C")
Review comment:
It should still work if we write `ID`, right? Spark normalizes the columns according to the table schema, so case sensitivity flag can still apply. See `ResolveAlterTableChanges`.
It would be better if we can add tests for case insensitive column resolution.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708929217
**[Test build #129808 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129808/testReport)** for PR 30041 at commit [`7e8098c`](https://github.com/apache/spark/commit/7e8098cebdfae077205253028e48d7b5bf9a8c07).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708897395
can we add a test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30041:
URL: https://github.com/apache/spark/pull/30041#issuecomment-708970478
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34413/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30041:
URL: https://github.com/apache/spark/pull/30041#discussion_r505202578
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -199,8 +199,8 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
test("alter table ... rename column") {
withTable("h2.test.alt_table") {
- sql("CREATE TABLE h2.test.alt_table (ID INTEGER, C0 INTEGER) USING _")
- sql("ALTER TABLE h2.test.alt_table RENAME COLUMN ID TO C")
+ sql("CREATE TABLE h2.test.alt_table (id INTEGER, C0 INTEGER) USING _")
+ sql("ALTER TABLE h2.test.alt_table RENAME COLUMN id TO C")
Review comment:
Without fix (with unquoted column name), this throws the following Exception:
```
Failed table altering: test.alt_table;
org.apache.spark.sql.AnalysisException: Failed table altering: test.alt_table;
at org.apache.spark.sql.jdbc.JdbcDialect.classifyException(JdbcDialects.scala:268)
Caused by: org.h2.jdbc.JdbcSQLException: Column "ID" not found; SQL statement:
ALTER TABLE "test"."alt_table" RENAME COLUMN id TO "C" [42122-195]
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #30041: [SPARK-32402][SQL][FOLLOW-UP] Use quoted column name for JDBCTableCatalog.alterTable
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #30041:
URL: https://github.com/apache/spark/pull/30041#discussion_r505202427
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -178,15 +178,15 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
.add("C1", IntegerType)
.add("C2", StringType)
assert(t.schema === expectedSchema)
- sql("ALTER TABLE h2.test.alt_table ADD COLUMNS (C3 DOUBLE)")
+ sql("ALTER TABLE h2.test.alt_table ADD COLUMNS (c3 DOUBLE)")
t = spark.table("h2.test.alt_table")
- expectedSchema = expectedSchema.add("C3", DoubleType)
+ expectedSchema = expectedSchema.add("c3", DoubleType)
Review comment:
Without fix (with unquoted column name), `assert(t.schema === expectedSchema)` throws the following Exception:
```
org.scalatest.exceptions.TestFailedException: StructType(StructField(ID,IntegerType,true), StructField(C1,IntegerType,true), StructField(C2,StringType,true), StructField(C3,DoubleType,true)) did not equal StructType(StructField(ID,IntegerType,true), StructField(C1,IntegerType,true), StructField(C2,StringType,true), StructField(c3,DoubleType,true))
at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org