Skip to content

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Apr 22, 2020

What changes were proposed in this pull request?

HiveClient instance is cross-session, the following configurations which are defined in HiveUtils and used to create it should be considered static:

  1. spark.sql.hive.metastore.version - used to determine the hive version in Spark
  2. spark.sql.hive.metastore.jars - hive metastore related jars location which is used by spark to create hive client
  3. spark.sql.hive.metastore.sharedPrefixes and spark.sql.hive.metastore.barrierPrefixes - package names of classes that are shared or separated between SparkContextLoader and hive client class loader

Those are used only once when creating the hive metastore client. They should be static in SQLConf for retrieving them correctly. We should avoid them being changed by users with SET/RESET command.

Speaking of spark.sql.hive.version - the fake of the spark.sql.hive.metastore.version, it is used by jdbc/thrift client for backward compatibility.

Why are the changes needed?

bugfix, these configurations should not be changed.

Does this PR introduce any user-facing change?

Yes, the following set of configs are not allowed to change.

Seq("spark.sql.hive.metastore.version ",
      "spark.sql.hive.metastore.jars",
      "spark.sql.hive.metastore.sharedPrefixes",
      "spark.sql.hive.metastore.barrierPrefixes")

How was this patch tested?

add unit test

@yaooqinn
Copy link
Member Author

cc: @cloud-fan @dongjoon-hyun @HyukjinKwon, many thanks.

@SparkQA
Copy link

SparkQA commented Apr 22, 2020

Test build #121635 has finished for PR 28302 at commit 344f558.

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

@SparkQA
Copy link

SparkQA commented Apr 22, 2020

Test build #121634 has finished for PR 28302 at commit d0cf0ec.

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

@SparkQA
Copy link

SparkQA commented Apr 23, 2020

Test build #121644 has finished for PR 28302 at commit 5c15a98.

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

@yaooqinn yaooqinn marked this pull request as draft April 23, 2020 05:34
@SparkQA
Copy link

SparkQA commented Apr 23, 2020

Test build #121652 has finished for PR 28302 at commit c72ba70.

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

@SparkQA
Copy link

SparkQA commented Apr 23, 2020

Test build #121650 has finished for PR 28302 at commit 09b87ff.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yaooqinn
Copy link
Member Author

retest this please

@SparkQA
Copy link

SparkQA commented Apr 23, 2020

Test build #121658 has finished for PR 28302 at commit c72ba70.

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

@SparkQA
Copy link

SparkQA commented Apr 23, 2020

Test build #121668 has finished for PR 28302 at commit 7a26872.

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

@yaooqinn yaooqinn marked this pull request as ready for review April 23, 2020 11:59
@cloud-fan
Copy link
Contributor

thanks, merging to master/3.0!

cc @juliuszsompolski as well.

@cloud-fan cloud-fan closed this in 8dc2c02 Apr 23, 2020
cloud-fan pushed a commit that referenced this pull request Apr 23, 2020
…gurations should be static

### What changes were proposed in this pull request?
HiveClient instance is cross-session, the following configurations which are defined in HiveUtils and used to create it should be considered static:

1. spark.sql.hive.metastore.version - used to determine the hive version in Spark
2. spark.sql.hive.metastore.jars - hive metastore related jars location which is used by spark to create hive client
3. spark.sql.hive.metastore.sharedPrefixes and spark.sql.hive.metastore.barrierPrefixes -  package names of classes that are shared or separated between SparkContextLoader and hive client class loader

Those are used only once when creating the hive metastore client. They should be static in SQLConf for retrieving them correctly. We should avoid them being changed by users with SET/RESET command.

Speaking of spark.sql.hive.version - the fake of the spark.sql.hive.metastore.version, it is used by jdbc/thrift client for backward compatibility.

### Why are the changes needed?

bugfix, these configurations should not be changed.

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

Yes, the following set of configs are not allowed to change.
```
Seq("spark.sql.hive.metastore.version ",
      "spark.sql.hive.metastore.jars",
      "spark.sql.hive.metastore.sharedPrefixes",
      "spark.sql.hive.metastore.barrierPrefixes")
```
### How was this patch tested?

add unit test

Closes #28302 from yaooqinn/SPARK-31522.

Authored-by: Kent Yao <yaooqinn@hotmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 8dc2c02)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@dongjoon-hyun
Copy link
Member

+1, late LGTM.

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.

4 participants