Skip to content

Conversation

@pan3793
Copy link
Member

@pan3793 pan3793 commented Sep 24, 2025

What changes were proposed in this pull request?

Previously, V1WriteHiveCommandSuite uses CREATE TABLE without declaring USING or STORED AS, which relies on spark.sql.legacy.createHiveTableByDefault's default value, SPARK-46122 changes the default value of spark.sql.legacy.createHiveTableByDefault from true to false, which implicitly affects this test suite.

In addition, we should take spark.sql.hive.convertMetastoreParquet and spark.sql.hive.convertMetastoreOrc into account in V1WriteHiveCommandSuite

Why are the changes needed?

Restore and improve test coverage.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GHA.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the SQL label Sep 24, 2025
@pan3793
Copy link
Member Author

pan3793 commented Sep 24, 2025

cc @cloud-fan

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the claims, why don't we use spark.sql.legacy.createHiveTableByDefault=(true|false) explicitly to improve the test coverage, @pan3793 ?

Previously, V1WriteHiveCommandSuite uses CREATE TABLE without declaring USING or STORED AS, which relies on spark.sql.legacy.createHiveTableByDefault's default value, SPARK-46122 changes the default value of spark.sql.legacy.createHiveTableByDefault from true to false, which implicitly affects this test suite.

@pan3793
Copy link
Member Author

pan3793 commented Sep 25, 2025

@dongjoon-hyun the test suite hierarchy is

trait V1WriteCommandSuiteBase ...
class V1WriteCommandSuite extends V1WriteCommandSuiteBase ...
class V1WriteHiveCommandSuite extends V1WriteCommandSuiteBase ...

V1WriteHiveCommandSuite intends to have only Hive SerDe table cases, so spark.sql.legacy.createHiveTableByDefault=false is unnecessary.

spark.sql.legacy.createHiveTableByDefault=true indirectly takes the same effect as STORED AS PARQUET here, but I prefer to use the latter instead of the indirect approach. BTW, in V1WriteCommandSuite, it explicitly uses USING PARQUET in the CREATE TABLE clause.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Got it., @pan3793 .

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-53694][SQL][TESTS] Improve V1WriteHiveCommandSuite test coverage [SPARK-53694][SQL][TESTS] Improve V1WriteHiveCommandSuite test coverage Sep 25, 2025
@dongjoon-hyun
Copy link
Member

Merged to master for Apache Spark 4.1.0-preview3.

pan3793 added a commit to pan3793/spark that referenced this pull request Oct 21, 2025
…rage

### What changes were proposed in this pull request?

Previously, `V1WriteHiveCommandSuite` uses `CREATE TABLE` without declaring `USING` or `STORED AS`, which relies on `spark.sql.legacy.createHiveTableByDefault`'s default value, SPARK-46122 changes the default value of `spark.sql.legacy.createHiveTableByDefault` from `true` to `false`, which implicitly affects this test suite.

In addition, we should take `spark.sql.hive.convertMetastoreParquet` and `spark.sql.hive.convertMetastoreOrc` into account in `V1WriteHiveCommandSuite`

### Why are the changes needed?

Restore and improve test coverage.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GHA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#52436 from pan3793/SPARK-53694.

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
peter-toth pushed a commit that referenced this pull request Oct 21, 2025
… coverage

Backport #52436 to branch-4.0 to recover CI.

### What changes were proposed in this pull request?

Previously, `V1WriteHiveCommandSuite` uses `CREATE TABLE` without declaring `USING` or `STORED AS`, which relies on `spark.sql.legacy.createHiveTableByDefault`'s default value, SPARK-46122 changes the default value of `spark.sql.legacy.createHiveTableByDefault` from `true` to `false`, which implicitly affects this test suite.

In addition, we should take `spark.sql.hive.convertMetastoreParquet` and `spark.sql.hive.convertMetastoreOrc` into account in `V1WriteHiveCommandSuite`

### Why are the changes needed?

Restore and improve test coverage.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GHA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #52683 from pan3793/SPARK-53694-4.0.

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Peter Toth <[email protected]>
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 14, 2025
… coverage

Backport apache#52436 to branch-4.0 to recover CI.

### What changes were proposed in this pull request?

Previously, `V1WriteHiveCommandSuite` uses `CREATE TABLE` without declaring `USING` or `STORED AS`, which relies on `spark.sql.legacy.createHiveTableByDefault`'s default value, SPARK-46122 changes the default value of `spark.sql.legacy.createHiveTableByDefault` from `true` to `false`, which implicitly affects this test suite.

In addition, we should take `spark.sql.hive.convertMetastoreParquet` and `spark.sql.hive.convertMetastoreOrc` into account in `V1WriteHiveCommandSuite`

### Why are the changes needed?

Restore and improve test coverage.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GHA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#52683 from pan3793/SPARK-53694-4.0.

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Peter Toth <[email protected]>
huangxiaopingRD pushed a commit to huangxiaopingRD/spark that referenced this pull request Nov 25, 2025
…rage

### What changes were proposed in this pull request?

Previously, `V1WriteHiveCommandSuite` uses `CREATE TABLE` without declaring `USING` or `STORED AS`, which relies on `spark.sql.legacy.createHiveTableByDefault`'s default value, SPARK-46122 changes the default value of `spark.sql.legacy.createHiveTableByDefault` from `true` to `false`, which implicitly affects this test suite.

In addition, we should take `spark.sql.hive.convertMetastoreParquet` and `spark.sql.hive.convertMetastoreOrc` into account in `V1WriteHiveCommandSuite`

### Why are the changes needed?

Restore and improve test coverage.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GHA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#52436 from pan3793/SPARK-53694.

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants