Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Oct 13, 2022

What changes were proposed in this pull request?

This pr add a sort operation to make the contents of SERDEPROPERTIES in the result of ShowCreateTableAsSerdeCommand have a fixed order.

Why are the changes needed?

Improve Java version compatibility, make the results of ShowCreateTableAsSerdeCommand consistent when using Java version.

Run the following command with Java 19:

mvn clean install -DskipTests -pl sql/hive -am  
mvn test -pl sql/hive -Dtest=none -DwildcardSuites=org.apache.spark.sql.hive.execution.command.ShowCreateTableSuite

There are 2 failed test:

- SHOW CREATE TABLE using Hive V1 catalog V1 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."
- SHOW CREATE TABLE using Hive V1 catalog V2 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."

The content of SERDEPROPERTIES is correct, but the order is different from result running with Java 8/11/17.

Does this PR introduce any user-facing change?

Yes, the SERDEPROPERTIES part of ShowCreateTableAsSerdeCommand results will be displayed in order by key.

How was this patch tested?

  • Pass Git Hub Actions
  • Manual test:

Run the following command with Java 19:

mvn clean install -DskipTests -pl sql/hive -am  
mvn test -pl sql/hive -Dtest=none -DwildcardSuites=org.apache.spark.sql.hive.execution.command.ShowCreateTableSuite

Before

- SHOW CREATE TABLE using Hive V1 catalog V1 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."
- SHOW CREATE TABLE using Hive V1 catalog V2 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."

After

Run completed in 23 seconds, 930 milliseconds.
Total number of tests run: 58
Suites: completed 2, aborted 0
Tests: succeeded 58, failed 0, canceled 0, ignored 0, pending 0
All tests passed.

@LuciferYang
Copy link
Contributor Author

Test first

@github-actions github-actions bot added the SQL label Oct 13, 2022
@LuciferYang LuciferYang changed the title [SPARK-40733][SQL] Make the contents of SERDEPROPERTIES in the ShowCreateTableAsSerdeCommand result have a fixed order [SPARK-40733][SQL] Make the contents of SERDEPROPERTIES in the result of ShowCreateTableAsSerdeCommand have a fixed order Oct 13, 2022
builder ++= s"ROW FORMAT SERDE '$serde'\n"

val serdeProps = conf.redactOptions(metadata.storage.properties).map {
val serdeProps = conf.redactOptions(metadata.storage.properties).toSeq.sortBy(_._1).map {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

private def showDataSourceTableOptions(metadata: CatalogTable, builder: StringBuilder): Unit = {
// For datasource table, there is a provider there in the metadata.
// If it is a Hive table, we already convert its metadata and fill in a provider.
builder ++= s"USING ${metadata.provider.get}\n"
val dataSourceOptions = conf.redactOptions(metadata.storage.properties).toSeq.sortBy(_._1).map {
case (key, value) =>
s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
}

showDataSourceTableOptions already do the similar work in #34753

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. Thank you, @LuciferYang .
Merged to master for Apache Spark 3.4.0.

@LuciferYang
Copy link
Contributor Author

Thanks @dongjoon-hyun

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
…lt of `ShowCreateTableAsSerdeCommand` have a fixed order

### What changes were proposed in this pull request?
This pr add a sort operation to make the contents of `SERDEPROPERTIES` in the result of `ShowCreateTableAsSerdeCommand` have a fixed order.

### Why are the changes needed?
Improve Java version compatibility, make the results of `ShowCreateTableAsSerdeCommand` consistent when using Java version.

Run the following command with Java 19:

```
mvn clean install -DskipTests -pl sql/hive -am
mvn test -pl sql/hive -Dtest=none -DwildcardSuites=org.apache.spark.sql.hive.execution.command.ShowCreateTableSuite
```

There are 2 failed test:

```
- SHOW CREATE TABLE using Hive V1 catalog V1 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."
- SHOW CREATE TABLE using Hive V1 catalog V2 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."
```

The content of `SERDEPROPERTIES` is correct, but the order is different from result running with Java 8/11/17.

### Does this PR introduce _any_ user-facing change?
Yes, the `SERDEPROPERTIES` part of `ShowCreateTableAsSerdeCommand` results will be displayed in order by key.

### How was this patch tested?

- Pass Git Hub Actions
- Manual test:

Run the following command with Java 19:

```
mvn clean install -DskipTests -pl sql/hive -am
mvn test -pl sql/hive -Dtest=none -DwildcardSuites=org.apache.spark.sql.hive.execution.command.ShowCreateTableSuite
```

**Before**

```
- SHOW CREATE TABLE using Hive V1 catalog V1 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."
- SHOW CREATE TABLE using Hive V1 catalog V2 command: hive table with serde info *** FAILED ***
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." did not equal "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..." (ShowCreateTableSuite.scala:187)
  Analysis:
  "... SERDEPROPERTIES ( '[serialization.format' = '1', 'field.delim' = ',', 'mapkey].delim' = ',') STORE..." -> "... SERDEPROPERTIES ( '[mapkey.delim' = ',', 'serialization.format' = '1', 'field].delim' = ',') STORE..."
```

**After**

```
Run completed in 23 seconds, 930 milliseconds.
Total number of tests run: 58
Suites: completed 2, aborted 0
Tests: succeeded 58, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
```

Closes apache#38238 from LuciferYang/SPARK-40733.

Authored-by: yangjie01 <[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.

2 participants