[SPARK-33770][SQL][TESTS] Fix the `ALTER TABLE .. DROP PARTITION` tests that delete files out of partition path #30752

MaxGekk · 2020-12-13T09:00:20Z

What changes were proposed in this pull request?

Modify the tests that add partitions with LOCATION, and where the number of nested folders in LOCATION doesn't match to the number of partitioned columns. In that case, ALTER TABLE .. DROP PARTITION tries to access (delete) folder out of the "base" path in LOCATION.

The problem belongs to Hive's MetaStore method drop_partition_common:
https://github.com/apache/hive/blob/8696c82d07d303b6dbb69b4d443ab6f2b241b251/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L4876
which tries to delete empty partition sub-folders recursively starting from the most deeper partition sub-folder up to the base folder. In the case when the number of sub-folder is not equal to the number of partitioned columns part_vals.size(), the method will try to list and delete folders out of the base path.

Why are the changes needed?

To fix test failures like #30643 (comment):

org.apache.spark.sql.hive.execution.command.AlterTableAddPartitionSuite.ALTER TABLE .. ADD PARTITION Hive V1: SPARK-33521: universal type conversions of partition values
sbt.ForkMain$ForkError: org.apache.spark.sql.AnalysisException: org.apache.hadoop.hive.ql.metadata.HiveException: File file:/home/jenkins/workspace/SparkPullRequestBuilder/target/tmp/spark-832cb19c-65fd-41f3-ae0b-937d76c07897 does not exist;
	at org.apache.spark.sql.hive.HiveExternalCatalog.withClient(HiveExternalCatalog.scala:112)
	at org.apache.spark.sql.hive.HiveExternalCatalog.dropPartitions(HiveExternalCatalog.scala:1014)
...
Caused by: sbt.ForkMain$ForkError: org.apache.hadoop.hive.metastore.api.MetaException: File file:/home/jenkins/workspace/SparkPullRequestBuilder/target/tmp/spark-832cb19c-65fd-41f3-ae0b-937d76c07897 does not exist
	at org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.drop_partition_with_environment_context(HiveMetaStore.java:3381)
	at sun.reflect.GeneratedMethodAccessor304.invoke(Unknown Source)

The issue can be reproduced by the following steps:

Create a base folder, for example: /Users/maximgekk/tmp/part-location
Create a sub-folder in the base folder and drop permissions for it:

$ mkdir /Users/maximgekk/tmp/part-location/aaa
$ chmod a-rwx chmod a-rwx /Users/maximgekk/tmp/part-location/aaa
$ ls -al /Users/maximgekk/tmp/part-location
total 0
drwxr-xr-x   3 maximgekk  staff    96 Dec 13 18:42 .
drwxr-xr-x  33 maximgekk  staff  1056 Dec 13 18:32 ..
d---------   2 maximgekk  staff    64 Dec 13 18:42 aaa

Create a table with a partition folder in the base folder:

spark-sql> create table tbl (id int) partitioned by (part0 int, part1 int);
spark-sql> alter table tbl add partition (part0=1,part1=2) location '/Users/maximgekk/tmp/part-location/tbl';

Try to drop this partition:

spark-sql> alter table tbl drop partition (part0=1,part1=2);
20/12/13 18:46:07 ERROR HiveClientImpl:
======================
Attempt to drop the partition specs in table 'tbl' database 'default':
Map(part0 -> 1, part1 -> 2)
In this attempt, the following partitions have been dropped successfully:

The remaining partitions have not been dropped:
[1, 2]
======================

Error in query: org.apache.hadoop.hive.ql.metadata.HiveException: Error accessing file:/Users/maximgekk/tmp/part-location/aaa;
org.apache.spark.sql.AnalysisException: org.apache.hadoop.hive.ql.metadata.HiveException: Error accessing file:/Users/maximgekk/tmp/part-location/aaa;

The command fails because it tries to access to the sub-folder aaa that is out of the partition path /Users/maximgekk/tmp/part-location/tbl.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running the affected tests from local IDEA which does not have access to folders out of partition paths.

…n values"

SparkQA · 2020-12-13T09:40:25Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37321/

SparkQA · 2020-12-13T10:06:53Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37321/

yaooqinn · 2020-12-13T10:27:28Z

thanks @MaxGekk

SparkQA · 2020-12-13T13:30:05Z

Test build #132718 has finished for PR 30752 at commit 4a9abab.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

MaxGekk · 2020-12-13T15:26:30Z

.../src/test/scala/org/apache/spark/sql/execution/command/AlterTableAddPartitionSuiteBase.scala

@@ -154,7 +154,7 @@ trait AlterTableAddPartitionSuiteBase extends QueryTest with SQLTestUtils {
        |  part8 = '2020-11-23',
        |  part9 = '2020-11-23 22:13:10.123456'
        |""".stripMargin
-      sql(s"ALTER TABLE $t ADD PARTITION ($partSpec) LOCATION 'loc1'")


location doesn't matter in the test.

dongjoon-hyun

Thanks, @MaxGekk . BTW, I have two questions.

Does the failure happen in Maven environment of master branch?
Does the failure happen in `branch-3.1?

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala

HyukjinKwon

The change itself looks good in any event.

…log/ExternalCatalogSuite.scala Co-authored-by: Hyukjin Kwon <[email protected]>

MaxGekk · 2020-12-14T05:48:55Z

Does the failure happen in Maven environment of master branch?

@dongjoon-hyun Yes, I got the failure when I imported the master as Maven project in IntelliJ IDEA locally.

Does the failure happen in `branch-3.1?

Yes, it suffers the same issue. The issue became more visible in master due to new test #30752 (comment) but other tests that I touched in this PR existed in branch-3.1 too.

SparkQA · 2020-12-14T05:58:36Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37347/

SparkQA · 2020-12-14T06:22:37Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37347/

MaxGekk · 2020-12-14T06:27:04Z

BTW, I reproduced the issue on branch-2.4 (sbt) in my local test environment. HiveExternalCatalogSuite fails:
the test "create/drop partitions in managed tables with location":

09:22:19.478 ERROR org.apache.spark.sql.hive.client.HiveClientImpl: 
======================
Attempt to drop the partition specs in table 'tbl' database 'db1':
Map(partcol1 -> 1, partcol2 -> 2)
In this attempt, the following partitions have been dropped successfully:

The remaining partitions have not been dropped:
[1, 2]
======================
             
org.apache.hadoop.hive.ql.metadata.HiveException: Error accessing file:/private/var/folders/p3/dfs6mf655d7fnjrsjvldh0tc0000gn/T/.../TemporaryItems;
org.apache.spark.sql.AnalysisException: org.apache.hadoop.hive.ql.metadata.HiveException: Error accessing file:/private/var/folders/p3/dfs6mf655d7fnjrsjvldh0tc0000gn/T/.../TemporaryItems;
	at org.apache.spark.sql.hive.HiveExternalCatalog.withClient(HiveExternalCatalog.scala:106)
	at org.apache.spark.sql.hive.HiveExternalCatalog.dropPartitions(HiveExternalCatalog.scala:970)
	at org.apache.spark.sql.catalyst.catalog.ExternalCatalogSuite$$anonfun$32.apply(ExternalCatalogSuite.scala:404)

HyukjinKwon · 2020-12-14T06:56:27Z

Merged to master.

HyukjinKwon · 2020-12-14T06:57:28Z

@MaxGekk it has a minor conflict against branch-3.1. Can you make a backporting PR?

…ts that delete files out of partition path Modify the tests that add partitions with `LOCATION`, and where the number of nested folders in `LOCATION` doesn't match to the number of partitioned columns. In that case, `ALTER TABLE .. DROP PARTITION` tries to access (delete) folder out of the "base" path in `LOCATION`. The problem belongs to Hive's MetaStore method `drop_partition_common`: https://github.com/apache/hive/blob/8696c82d07d303b6dbb69b4d443ab6f2b241b251/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L4876 which tries to delete empty partition sub-folders recursively starting from the most deeper partition sub-folder up to the base folder. In the case when the number of sub-folder is not equal to the number of partitioned columns `part_vals.size()`, the method will try to list and delete folders out of the base path. To fix test failures like apache#30643 (comment): ``` org.apache.spark.sql.hive.execution.command.AlterTableAddPartitionSuite.ALTER TABLE .. ADD PARTITION Hive V1: SPARK-33521: universal type conversions of partition values sbt.ForkMain$ForkError: org.apache.spark.sql.AnalysisException: org.apache.hadoop.hive.ql.metadata.HiveException: File file:/home/jenkins/workspace/SparkPullRequestBuilder/target/tmp/spark-832cb19c-65fd-41f3-ae0b-937d76c07897 does not exist; at org.apache.spark.sql.hive.HiveExternalCatalog.withClient(HiveExternalCatalog.scala:112) at org.apache.spark.sql.hive.HiveExternalCatalog.dropPartitions(HiveExternalCatalog.scala:1014) ... Caused by: sbt.ForkMain$ForkError: org.apache.hadoop.hive.metastore.api.MetaException: File file:/home/jenkins/workspace/SparkPullRequestBuilder/target/tmp/spark-832cb19c-65fd-41f3-ae0b-937d76c07897 does not exist at org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.drop_partition_with_environment_context(HiveMetaStore.java:3381) at sun.reflect.GeneratedMethodAccessor304.invoke(Unknown Source) ``` The issue can be reproduced by the following steps: 1. Create a base folder, for example: `/Users/maximgekk/tmp/part-location` 2. Create a sub-folder in the base folder and drop permissions for it: ``` $ mkdir /Users/maximgekk/tmp/part-location/aaa $ chmod a-rwx chmod a-rwx /Users/maximgekk/tmp/part-location/aaa $ ls -al /Users/maximgekk/tmp/part-location total 0 drwxr-xr-x 3 maximgekk staff 96 Dec 13 18:42 . drwxr-xr-x 33 maximgekk staff 1056 Dec 13 18:32 .. d--------- 2 maximgekk staff 64 Dec 13 18:42 aaa ``` 3. Create a table with a partition folder in the base folder: ```sql spark-sql> create table tbl (id int) partitioned by (part0 int, part1 int); spark-sql> alter table tbl add partition (part0=1,part1=2) location '/Users/maximgekk/tmp/part-location/tbl'; ``` 4. Try to drop this partition: ``` spark-sql> alter table tbl drop partition (part0=1,part1=2); 20/12/13 18:46:07 ERROR HiveClientImpl: ====================== Attempt to drop the partition specs in table 'tbl' database 'default': Map(part0 -> 1, part1 -> 2) In this attempt, the following partitions have been dropped successfully: The remaining partitions have not been dropped: [1, 2] ====================== Error in query: org.apache.hadoop.hive.ql.metadata.HiveException: Error accessing file:/Users/maximgekk/tmp/part-location/aaa; org.apache.spark.sql.AnalysisException: org.apache.hadoop.hive.ql.metadata.HiveException: Error accessing file:/Users/maximgekk/tmp/part-location/aaa; ``` The command fails because it tries to access to the sub-folder `aaa` that is out of the partition path `/Users/maximgekk/tmp/part-location/tbl`. No By running the affected tests from local IDEA which does not have access to folders out of partition paths. Closes apache#30752 from MaxGekk/fix-drop-partition-location. Lead-authored-by: Max Gekk <[email protected]> Co-authored-by: Maxim Gekk <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit 9160d59) Signed-off-by: Max Gekk <[email protected]>

MaxGekk · 2020-12-14T07:59:12Z

Here are backports to:

branch-3.1/branch-3.0: [SPARK-33770][SQL][TESTS][3.1][3.0] Fix the ALTER TABLE .. DROP PARTITION tests that delete files out of partition path #30756
branch-2.4: [SPARK-33770][SQL][TESTS][2.4] Fix the ALTER TABLE .. DROP PARTITION tests that delete files out of partition path #30757

SparkQA · 2020-12-14T09:53:47Z

Test build #132746 has finished for PR 30752 at commit e9a6133.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2022-05-09T00:31:45Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala

-                 |PARTITION (ds='2008-04-09', hr='11') LOCATION '${dir1.toURI.toString}'
-                 |PARTITION (ds='2008-04-09', hr='12') LOCATION '${dir2.toURI.toString}'
+                 |PARTITION (ds='2008-04-09', hr='11') LOCATION '${partDir1.toURI.toString}'
+                 |PARTITION (ds='2008-04-09', hr='12') LOCATION '${partDir1.toURI.toString}'


Sorry for commenting here.

This bug is introduced here and backported to 2.4.8/3.0.2/3.1.0 and SPARK-38786 fixed this at master branch for Apache Spark 3.4 recently.

MaxGekk added 4 commits December 13, 2020 09:52

Remove LOCATION from the test "universal type conversions of partitio…

5439710

…n values"

Fix the test "create/drop partitions in managed tables with location"

4da34f2

Change paths in "add/drop partition with location - managed table"

8916218

Fix StatisticsSuite

4a9abab

github-actions bot added the SQL label Dec 13, 2020

MaxGekk changed the title ~~[WIP][SQL] Avoid access to files out of partition dirs in ALTER TABLE .. DROP PARTITION~~ [WIP][SQL][TESTS] Avoid access to files out of partition dirs in ALTER TABLE .. DROP PARTITION Dec 13, 2020

MaxGekk changed the title ~~[WIP][SQL][TESTS] Avoid access to files out of partition dirs in ALTER TABLE .. DROP PARTITION~~ [SPARK-33770][SQL][TESTS] Fix the ALTER TABLE .. DROP PARTITION tests that delete files out of partition path Dec 13, 2020

MaxGekk commented Dec 13, 2020

View reviewed changes

dongjoon-hyun reviewed Dec 13, 2020

View reviewed changes

HyukjinKwon reviewed Dec 14, 2020

View reviewed changes

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala Outdated Show resolved Hide resolved

HyukjinKwon approved these changes Dec 14, 2020

View reviewed changes

Update sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/cata…

e9a6133

…log/ExternalCatalogSuite.scala Co-authored-by: Hyukjin Kwon <[email protected]>

HyukjinKwon closed this in 9160d59 Dec 14, 2020

MaxGekk deleted the fix-drop-partition-location branch February 19, 2021 15:04

dongjoon-hyun reviewed May 9, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-33770][SQL][TESTS] Fix the `ALTER TABLE .. DROP PARTITION` tests that delete files out of partition path #30752

[SPARK-33770][SQL][TESTS] Fix the `ALTER TABLE .. DROP PARTITION` tests that delete files out of partition path #30752

MaxGekk commented Dec 13, 2020 •

edited

Loading

SparkQA commented Dec 13, 2020

SparkQA commented Dec 13, 2020

yaooqinn commented Dec 13, 2020

SparkQA commented Dec 13, 2020

MaxGekk Dec 13, 2020

dongjoon-hyun left a comment

HyukjinKwon left a comment

MaxGekk commented Dec 14, 2020

SparkQA commented Dec 14, 2020

SparkQA commented Dec 14, 2020

MaxGekk commented Dec 14, 2020

HyukjinKwon commented Dec 14, 2020 •

edited

Loading

HyukjinKwon commented Dec 14, 2020

MaxGekk commented Dec 14, 2020

SparkQA commented Dec 14, 2020

dongjoon-hyun May 9, 2022

[SPARK-33770][SQL][TESTS] Fix the ALTER TABLE .. DROP PARTITION tests that delete files out of partition path #30752

[SPARK-33770][SQL][TESTS] Fix the ALTER TABLE .. DROP PARTITION tests that delete files out of partition path #30752

Conversation

MaxGekk commented Dec 13, 2020 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Dec 13, 2020

SparkQA commented Dec 13, 2020

yaooqinn commented Dec 13, 2020

SparkQA commented Dec 13, 2020

MaxGekk Dec 13, 2020

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

HyukjinKwon left a comment

Choose a reason for hiding this comment

MaxGekk commented Dec 14, 2020

SparkQA commented Dec 14, 2020

SparkQA commented Dec 14, 2020

MaxGekk commented Dec 14, 2020

HyukjinKwon commented Dec 14, 2020 • edited Loading

HyukjinKwon commented Dec 14, 2020

MaxGekk commented Dec 14, 2020

SparkQA commented Dec 14, 2020

dongjoon-hyun May 9, 2022

Choose a reason for hiding this comment

[SPARK-33770][SQL][TESTS] Fix the `ALTER TABLE .. DROP PARTITION` tests that delete files out of partition path #30752

[SPARK-33770][SQL][TESTS] Fix the `ALTER TABLE .. DROP PARTITION` tests that delete files out of partition path #30752

MaxGekk commented Dec 13, 2020 •

edited

Loading

HyukjinKwon commented Dec 14, 2020 •

edited

Loading