-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-28331][SQL] Catalogs.load() should be able to load built-in catalogs #25094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Test build #107454 has finished for PR 25094 at commit
|
| public static CatalogPlugin load(String name, SQLConf conf) | ||
| throws CatalogNotFoundException, SparkException { | ||
| String pluginClassName = conf.getConfString("spark.sql.catalog." + name, null); | ||
| if (pluginClassName == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a regression test case for this?
|
Let's wait on this. The problem it fixes hasn't been committed yet. |
|
cc @gengliangwang Do you have time to update this? |
|
Also, do we need both this and #25104? |
|
retest this please |
|
Test build #108677 has finished for PR 25094 at commit
|
|
LGTM. Merging this to master. Tests in @cloud-fan's refactoring should catch these. |
What changes were proposed in this pull request?
In
Catalogs.load, thepluginClassNamein the following codeis always null for built-in catalogs, e.g there is a SQLConf entry
spark.sql.catalog.session.This is because of #18852: SQLConf.conf.getConfString(key, null) always returns null.
How was this patch tested?
Apply code changes of #24768 and tried loading session catalog.