-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-34543][SQL] Respect the spark.sql.caseSensitive
config while resolving partition spec in v1 SET LOCATION
#31651
Conversation
@dongjoon-hyun @HyukjinKwon Could you review this PR. It fixes similar issue as #30615 which you have reviewed already. |
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #135474 has finished for PR 31651 at commit
|
jenkins, retest this, please |
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #135476 has finished for PR 31651 at commit
|
@cloud-fan May I ask you to take a look at this small fix. |
… resolving partition spec in v1 `SET LOCATION` ### What changes were proposed in this pull request? Preprocess the partition spec passed to the V1 `ALTER TABLE .. SET LOCATION` implementation `AlterTableSetLocationCommand`, and normalize the passed spec according to the partition columns w.r.t the case sensitivity flag **spark.sql.caseSensitive**. ### Why are the changes needed? V1 `ALTER TABLE .. SET LOCATION` is case sensitive in fact, and doesn't respect the SQL config **spark.sql.caseSensitive** which is false by default, for instance: ```sql spark-sql> CREATE TABLE tbl (id INT, part INT) PARTITIONED BY (part); spark-sql> INSERT INTO tbl PARTITION (part=0) SELECT 0; spark-sql> SHOW TABLE EXTENDED LIKE 'tbl' PARTITION (part=0); Location: file:/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 spark-sql> ALTER TABLE tbl ADD PARTITION (part=1); spark-sql> SELECT * FROM tbl; 0 0 ``` Create new partition folder in the file system: ``` $ cp -r /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/aaa ``` Set new location for the partition part=1: ```sql spark-sql> ALTER TABLE tbl PARTITION (part=1) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/aaa'; spark-sql> SELECT * FROM tbl; 0 0 0 1 spark-sql> ALTER TABLE tbl ADD PARTITION (PART=2); spark-sql> SELECT * FROM tbl; 0 0 0 1 ``` Set location for a partition in the upper case: ``` $ cp -r /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb ``` ```sql spark-sql> ALTER TABLE tbl PARTITION (PART=2) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb'; Error in query: Partition spec is invalid. The spec (PART) must match the partition spec (part) defined in table '`default`.`tbl`' ``` ### Does this PR introduce _any_ user-facing change? Yes. After the changes, the command above works as expected: ```sql spark-sql> ALTER TABLE tbl PARTITION (PART=2) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb'; spark-sql> SELECT * FROM tbl; 0 0 0 1 0 2 ``` ### How was this patch tested? By running the modified test suite: ``` $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *CatalogedDDLSuite" ``` Closes #31651 from MaxGekk/set-location-case-sense. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 5c7d019) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
… resolving partition spec in v1 `SET LOCATION` ### What changes were proposed in this pull request? Preprocess the partition spec passed to the V1 `ALTER TABLE .. SET LOCATION` implementation `AlterTableSetLocationCommand`, and normalize the passed spec according to the partition columns w.r.t the case sensitivity flag **spark.sql.caseSensitive**. ### Why are the changes needed? V1 `ALTER TABLE .. SET LOCATION` is case sensitive in fact, and doesn't respect the SQL config **spark.sql.caseSensitive** which is false by default, for instance: ```sql spark-sql> CREATE TABLE tbl (id INT, part INT) PARTITIONED BY (part); spark-sql> INSERT INTO tbl PARTITION (part=0) SELECT 0; spark-sql> SHOW TABLE EXTENDED LIKE 'tbl' PARTITION (part=0); Location: file:/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 spark-sql> ALTER TABLE tbl ADD PARTITION (part=1); spark-sql> SELECT * FROM tbl; 0 0 ``` Create new partition folder in the file system: ``` $ cp -r /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/aaa ``` Set new location for the partition part=1: ```sql spark-sql> ALTER TABLE tbl PARTITION (part=1) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/aaa'; spark-sql> SELECT * FROM tbl; 0 0 0 1 spark-sql> ALTER TABLE tbl ADD PARTITION (PART=2); spark-sql> SELECT * FROM tbl; 0 0 0 1 ``` Set location for a partition in the upper case: ``` $ cp -r /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb ``` ```sql spark-sql> ALTER TABLE tbl PARTITION (PART=2) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb'; Error in query: Partition spec is invalid. The spec (PART) must match the partition spec (part) defined in table '`default`.`tbl`' ``` ### Does this PR introduce _any_ user-facing change? Yes. After the changes, the command above works as expected: ```sql spark-sql> ALTER TABLE tbl PARTITION (PART=2) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb'; spark-sql> SELECT * FROM tbl; 0 0 0 1 0 2 ``` ### How was this patch tested? By running the modified test suite: ``` $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *CatalogedDDLSuite" ``` Closes #31651 from MaxGekk/set-location-case-sense. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 5c7d019) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
thanks, merging to master/3.1/3.0! |
Hi, guys.
|
… resolving partition spec in v1 `SET LOCATION` Preprocess the partition spec passed to the V1 `ALTER TABLE .. SET LOCATION` implementation `AlterTableSetLocationCommand`, and normalize the passed spec according to the partition columns w.r.t the case sensitivity flag **spark.sql.caseSensitive**. V1 `ALTER TABLE .. SET LOCATION` is case sensitive in fact, and doesn't respect the SQL config **spark.sql.caseSensitive** which is false by default, for instance: ```sql spark-sql> CREATE TABLE tbl (id INT, part INT) PARTITIONED BY (part); spark-sql> INSERT INTO tbl PARTITION (part=0) SELECT 0; spark-sql> SHOW TABLE EXTENDED LIKE 'tbl' PARTITION (part=0); Location: file:/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 spark-sql> ALTER TABLE tbl ADD PARTITION (part=1); spark-sql> SELECT * FROM tbl; 0 0 ``` Create new partition folder in the file system: ``` $ cp -r /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/aaa ``` Set new location for the partition part=1: ```sql spark-sql> ALTER TABLE tbl PARTITION (part=1) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/aaa'; spark-sql> SELECT * FROM tbl; 0 0 0 1 spark-sql> ALTER TABLE tbl ADD PARTITION (PART=2); spark-sql> SELECT * FROM tbl; 0 0 0 1 ``` Set location for a partition in the upper case: ``` $ cp -r /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/part=0 /Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb ``` ```sql spark-sql> ALTER TABLE tbl PARTITION (PART=2) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb'; Error in query: Partition spec is invalid. The spec (PART) must match the partition spec (part) defined in table '`default`.`tbl`' ``` Yes. After the changes, the command above works as expected: ```sql spark-sql> ALTER TABLE tbl PARTITION (PART=2) SET LOCATION '/Users/maximgekk/proj/set-location-case-sense/spark-warehouse/tbl/bbb'; spark-sql> SELECT * FROM tbl; 0 0 0 1 0 2 ``` By running the modified test suite: ``` $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *CatalogedDDLSuite" ``` Closes apache#31651 from MaxGekk/set-location-case-sense. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 5c7d019) Signed-off-by: Max Gekk <max.gekk@gmail.com>
Here is the backport to |
Thank you, @MaxGekk . |
What changes were proposed in this pull request?
Preprocess the partition spec passed to the V1
ALTER TABLE .. SET LOCATION
implementationAlterTableSetLocationCommand
, and normalize the passed spec according to the partition columns w.r.t the case sensitivity flag spark.sql.caseSensitive.Why are the changes needed?
V1
ALTER TABLE .. SET LOCATION
is case sensitive in fact, and doesn't respect the SQL config spark.sql.caseSensitive which is false by default, for instance:Create new partition folder in the file system:
Set new location for the partition part=1:
Set location for a partition in the upper case:
Does this PR introduce any user-facing change?
Yes. After the changes, the command above works as expected:
How was this patch tested?
By running the modified test suite: