[SPARK-22779][sql] Resolve default values for fallback configs. #19974
Conversation
SQLConf allows some callers to define a custom default value for configs, and that complicates a little bit the handling of fallback config entries, since most of the default value resolution is hidden by the config code. This change peaks into the internals of these fallback configs to figure out the correct default value, and also returns the current human-readable default when showing the default value (e.g. through "set -v").
|
@rxin I believe this is what you were trying to do? |
|
Test build #84890 has finished for PR 19974 at commit
|
|
|
||
| test("SPARK-22779: correctly compute default value for fallback configs") { | ||
| val fallback = SQLConf.buildConf("spark.sql.__test__.spark_22779") | ||
| .fallbackConf(SQLConf.PARQUET_COMPRESSION) |
There was a problem hiding this comment.
In SQLConf, all our conf are TypedConfigBuilder, instead of ConfigBuilder. The type-safe ConfigBuilder is unable to call fallbackConf , right?
There was a problem hiding this comment.
Yea sounds like this PR is fixing a non-existing issue.
There was a problem hiding this comment.
It's true no existing config uses this, but Reynold was trying to fix this so my guess is he's trying to add one and it wasn't working as expected.
There was a problem hiding this comment.
I think at least this is good for renaming. In the future, if we rename the SQLConf, we can use this mechanism.
|
LGTM |
|
Thanks! Merged to master. |
4 participants
SQLConf allows some callers to define a custom default value for
configs, and that complicates a little bit the handling of fallback
config entries, since most of the default value resolution is
hidden by the config code.
This change peaks into the internals of these fallback configs
to figure out the correct default value, and also returns the
current human-readable default when showing the default value
(e.g. through "set -v").