-
Notifications
You must be signed in to change notification settings - Fork 3k
DO NOT MERGE WILL BREAK - Change BaseCatalog to Interface #11210
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
Closed
RussellSpitzer
wants to merge
1
commit into
apache:main
from
RussellSpitzer:Spark353SparkSessionBreak
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ | |
| import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException; | ||
| import org.apache.spark.sql.connector.catalog.CatalogExtension; | ||
| import org.apache.spark.sql.connector.catalog.CatalogPlugin; | ||
| import org.apache.spark.sql.connector.catalog.DelegatingCatalogExtension; | ||
| import org.apache.spark.sql.connector.catalog.FunctionCatalog; | ||
| import org.apache.spark.sql.connector.catalog.Identifier; | ||
| import org.apache.spark.sql.connector.catalog.NamespaceChange; | ||
|
|
@@ -56,13 +57,12 @@ | |
| * SupportsNamespaces. | ||
| */ | ||
| public class SparkSessionCatalog<T extends TableCatalog & FunctionCatalog & SupportsNamespaces> | ||
| extends BaseCatalog implements CatalogExtension { | ||
| extends DelegatingCatalogExtension implements BaseCatalog { | ||
| private static final String[] DEFAULT_NAMESPACE = new String[] {"default"}; | ||
|
|
||
| private String catalogName = null; | ||
| private TableCatalog icebergCatalog = null; | ||
| private StagingTableCatalog asStagingCatalog = null; | ||
| private T sessionCatalog = null; | ||
| private boolean createParquetAsIceberg = false; | ||
| private boolean createAvroAsIceberg = false; | ||
| private boolean createOrcAsIceberg = false; | ||
|
|
@@ -88,57 +88,12 @@ public String[] defaultNamespace() { | |
| return DEFAULT_NAMESPACE; | ||
| } | ||
|
|
||
| @Override | ||
| public String[][] listNamespaces() throws NoSuchNamespaceException { | ||
| return getSessionCatalog().listNamespaces(); | ||
| } | ||
|
|
||
| @Override | ||
| public String[][] listNamespaces(String[] namespace) throws NoSuchNamespaceException { | ||
| return getSessionCatalog().listNamespaces(namespace); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean namespaceExists(String[] namespace) { | ||
| return getSessionCatalog().namespaceExists(namespace); | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, String> loadNamespaceMetadata(String[] namespace) | ||
| throws NoSuchNamespaceException { | ||
| return getSessionCatalog().loadNamespaceMetadata(namespace); | ||
| } | ||
|
|
||
| @Override | ||
| public void createNamespace(String[] namespace, Map<String, String> metadata) | ||
| throws NamespaceAlreadyExistsException { | ||
| getSessionCatalog().createNamespace(namespace, metadata); | ||
| } | ||
|
|
||
| @Override | ||
| public void alterNamespace(String[] namespace, NamespaceChange... changes) | ||
| throws NoSuchNamespaceException { | ||
| getSessionCatalog().alterNamespace(namespace, changes); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean dropNamespace(String[] namespace, boolean cascade) | ||
| throws NoSuchNamespaceException, NonEmptyNamespaceException { | ||
| return getSessionCatalog().dropNamespace(namespace, cascade); | ||
| } | ||
|
|
||
| @Override | ||
| public Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException { | ||
| // delegate to the session catalog because all tables share the same namespace | ||
| return getSessionCatalog().listTables(namespace); | ||
| } | ||
|
|
||
| @Override | ||
| public Table loadTable(Identifier ident) throws NoSuchTableException { | ||
| try { | ||
| return icebergCatalog.loadTable(ident); | ||
| } catch (NoSuchTableException e) { | ||
| return getSessionCatalog().loadTable(ident); | ||
| return super.loadTable(ident); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -147,7 +102,7 @@ public Table loadTable(Identifier ident, String version) throws NoSuchTableExcep | |
| try { | ||
| return icebergCatalog.loadTable(ident, version); | ||
| } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { | ||
| return getSessionCatalog().loadTable(ident, version); | ||
| return super.loadTable(ident, version); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -156,7 +111,7 @@ public Table loadTable(Identifier ident, long timestamp) throws NoSuchTableExcep | |
| try { | ||
| return icebergCatalog.loadTable(ident, timestamp); | ||
| } catch (org.apache.iceberg.exceptions.NoSuchTableException e) { | ||
| return getSessionCatalog().loadTable(ident, timestamp); | ||
| return super.loadTable(ident, timestamp); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -165,7 +120,7 @@ public void invalidateTable(Identifier ident) { | |
| // We do not need to check whether the table exists and whether | ||
| // it is an Iceberg table to reduce remote service requests. | ||
| icebergCatalog.invalidateTable(ident); | ||
| getSessionCatalog().invalidateTable(ident); | ||
| super.invalidateTable(ident); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -177,7 +132,7 @@ public Table createTable( | |
| return icebergCatalog.createTable(ident, schema, partitions, properties); | ||
| } else { | ||
| // delegate to the session catalog | ||
| return getSessionCatalog().createTable(ident, schema, partitions, properties); | ||
| return super.createTable(ident, schema, partitions, properties); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -193,7 +148,7 @@ public StagedTable stageCreate( | |
| } | ||
| catalog = icebergCatalog; | ||
| } else { | ||
| catalog = getSessionCatalog(); | ||
| throw new UnsupportedOperationException("Cannot stage a table create on the Session Catalog"); | ||
| } | ||
|
|
||
| // create the table with the session catalog, then wrap it in a staged table that will delete to | ||
|
|
@@ -214,7 +169,7 @@ public StagedTable stageReplace( | |
| } | ||
| catalog = icebergCatalog; | ||
| } else { | ||
| catalog = getSessionCatalog(); | ||
| throw new UnsupportedOperationException("Cannot stage a table replace on the Session Catalog"); | ||
| } | ||
|
|
||
| // attempt to drop the table and fail if it doesn't exist | ||
|
|
@@ -246,7 +201,7 @@ public StagedTable stageCreateOrReplace( | |
| } | ||
| catalog = icebergCatalog; | ||
| } else { | ||
| catalog = getSessionCatalog(); | ||
| throw new UnsupportedOperationException("Cannot stage a table create or replace on the Session Catalog"); | ||
| } | ||
|
|
||
| // drop the table if it exists | ||
|
|
@@ -269,7 +224,7 @@ public Table alterTable(Identifier ident, TableChange... changes) throws NoSuchT | |
| if (icebergCatalog.tableExists(ident)) { | ||
| return icebergCatalog.alterTable(ident, changes); | ||
| } else { | ||
| return getSessionCatalog().alterTable(ident, changes); | ||
| return super.alterTable(ident, changes); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -278,15 +233,15 @@ public boolean dropTable(Identifier ident) { | |
| // no need to check table existence to determine which catalog to use. if a table doesn't exist | ||
| // then both are | ||
| // required to return false. | ||
| return icebergCatalog.dropTable(ident) || getSessionCatalog().dropTable(ident); | ||
| return icebergCatalog.dropTable(ident) || super.dropTable(ident); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean purgeTable(Identifier ident) { | ||
| // no need to check table existence to determine which catalog to use. if a table doesn't exist | ||
| // then both are | ||
| // required to return false. | ||
| return icebergCatalog.purgeTable(ident) || getSessionCatalog().purgeTable(ident); | ||
| return icebergCatalog.purgeTable(ident) || super.purgeTable(ident); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -298,14 +253,16 @@ public void renameTable(Identifier from, Identifier to) | |
| if (icebergCatalog.tableExists(from)) { | ||
| icebergCatalog.renameTable(from, to); | ||
| } else { | ||
| getSessionCatalog().renameTable(from, to); | ||
| super.renameTable(from, to); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Removed | ||
| * | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Problem 2. We break all of our Catalog Config code |
||
| * | ||
| @Override | ||
| public final void initialize(String name, CaseInsensitiveStringMap options) { | ||
| super.initialize(name, options); | ||
|
|
||
| if (options.containsKey(CatalogUtil.ICEBERG_CATALOG_TYPE) | ||
| && options | ||
| .get(CatalogUtil.ICEBERG_CATALOG_TYPE) | ||
|
|
@@ -323,6 +280,7 @@ public final void initialize(String name, CaseInsensitiveStringMap options) { | |
| this.createAvroAsIceberg = options.getBoolean("avro-enabled", createAvroAsIceberg); | ||
| this.createOrcAsIceberg = options.getBoolean("orc-enabled", createOrcAsIceberg); | ||
| } | ||
| **/ | ||
|
|
||
| private void validateHmsUri(String catalogHmsUri) { | ||
| if (catalogHmsUri == null) { | ||
|
|
@@ -342,18 +300,6 @@ private void validateHmsUri(String catalogHmsUri) { | |
| catalogHmsUri); | ||
| } | ||
|
|
||
| @Override | ||
| @SuppressWarnings("unchecked") | ||
| public void setDelegateCatalog(CatalogPlugin sparkSessionCatalog) { | ||
| if (sparkSessionCatalog instanceof TableCatalog | ||
| && sparkSessionCatalog instanceof FunctionCatalog | ||
| && sparkSessionCatalog instanceof SupportsNamespaces) { | ||
| this.sessionCatalog = (T) sparkSessionCatalog; | ||
| } else { | ||
| throw new IllegalArgumentException("Invalid session catalog: " + sparkSessionCatalog); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String name() { | ||
| return catalogName; | ||
|
|
@@ -373,13 +319,6 @@ private boolean useIceberg(String provider) { | |
| return false; | ||
| } | ||
|
|
||
| private T getSessionCatalog() { | ||
| Preconditions.checkNotNull( | ||
| sessionCatalog, | ||
| "Delegated SessionCatalog is missing. " | ||
| + "Please make sure your are replacing Spark's default catalog, named 'spark_catalog'."); | ||
| return sessionCatalog; | ||
| } | ||
|
|
||
| @Override | ||
| public Catalog icebergCatalog() { | ||
|
|
@@ -392,9 +331,9 @@ public Catalog icebergCatalog() { | |
| @Override | ||
| public UnboundFunction loadFunction(Identifier ident) throws NoSuchFunctionException { | ||
| try { | ||
| return super.loadFunction(ident); | ||
| return loadFunction(ident); | ||
| } catch (NoSuchFunctionException e) { | ||
| return getSessionCatalog().loadFunction(ident); | ||
| return super.loadFunction(ident); | ||
| } | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Problem 1. Cannot do our old staging behavior without rewriting considerably more