twalthr commented on a change in pull request #15245:
URL: https://github.com/apache/flink/pull/15245#discussion_r595817618
##########
File path: docs/content/docs/dev/table/catalogs.md
##########
@@ -61,10 +61,9 @@ Flink's [Hive documentation]({{< ref
"docs/connectors/table/hive/overview" >}})
### User-Defined Catalog
Catalogs are pluggable and users can develop custom catalogs by implementing
the `Catalog` interface.
-To use custom catalogs in SQL CLI, users should develop both a catalog and its
corresponding catalog factory by implementing the `CatalogFactory` interface.
-The catalog factory defines a set of properties for configuring the catalog
when the SQL CLI bootstraps.
-The set of properties will be passed to a discovery service where the service
tries to match the properties to a `CatalogFactory` and initiate a
corresponding catalog instance.
+In order to use custom catalogs with Flink SQL, users should implement a
corresponding catalog factory by implementing the `CatalogFactory` interface.
Review comment:
```
The factory is discovered using Java's Service Provider Interfaces (SPI).
Classes that implement this interface can be added to
`META_INF/services/org.apache.flink.table.factories.Factory` in JAR files.
```
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java
##########
@@ -19,22 +19,103 @@
package org.apache.flink.table.factories;
import org.apache.flink.annotation.PublicEvolving;
+import org.apache.flink.configuration.ConfigOption;
+import org.apache.flink.configuration.ReadableConfig;
import org.apache.flink.table.catalog.Catalog;
+import org.apache.flink.table.catalog.exceptions.CatalogException;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
* A factory to create configured catalog instances based on string-based
properties. See also
- * {@link TableFactory} for more information.
+ * {@link Factory} for more information.
+ *
+ * <p>Note that this interface supports the {@link TableFactory} stack for
compatibility purposes.
+ * This is deprecated, however, and new implementations should implement the
{@link Factory} stack
+ * instead.
*/
@PublicEvolving
-public interface CatalogFactory extends TableFactory {
+public interface CatalogFactory extends TableFactory, Factory {
/**
* Creates and configures a {@link Catalog} using the given properties.
*
* @param properties normalized properties describing an external catalog.
* @return the configured catalog.
+ * @deprecated Use {@link this#createCatalog(Context)} instead and
implement {@link Factory}
+ * instead of {@link TableFactory}.
+ */
+ @Deprecated
+ default Catalog createCatalog(String name, Map<String, String> properties)
{
+ throw new CatalogException("Catalog factories must implement
createCatalog()");
+ }
+
+ /**
+ * Creates and configures a {@link Catalog} using the given context.
+ *
+ * <p>An implementation should perform validation and the discovery of
further (nested)
+ * factories in this method.
*/
- Catalog createCatalog(String name, Map<String, String> properties);
+ default Catalog createCatalog(Context context) {
+ throw new CatalogException("Catalog factories must implement
createCatalog(Context)");
+ }
+
+ /** Context provided when a catalog is created. */
+ interface Context {
+ /** Returns the name with which the catalog is created. */
+ String getName();
+
+ /**
+ * Returns the properties with which the catalog is created.
+ *
+ * <p>An implementation should perform validation of these properties.
+ */
+ Map<String, String> getProperties();
+
+ /** Gives read-only access to the configuration of the current
session. */
+ ReadableConfig getConfiguration();
+
+ /**
+ * Returns the class loader of the current session.
+ *
+ * <p>The class loader is in particular useful for discovering further
(nested) factories.
+ */
+ ClassLoader getClassLoader();
+ }
+
+ default Map<String, String> requiredContext() {
Review comment:
deprecate this method and below method
##########
File path: docs/content/docs/dev/table/catalogs.md
##########
@@ -61,10 +61,9 @@ Flink's [Hive documentation]({{< ref
"docs/connectors/table/hive/overview" >}})
### User-Defined Catalog
Catalogs are pluggable and users can develop custom catalogs by implementing
the `Catalog` interface.
-To use custom catalogs in SQL CLI, users should develop both a catalog and its
corresponding catalog factory by implementing the `CatalogFactory` interface.
-The catalog factory defines a set of properties for configuring the catalog
when the SQL CLI bootstraps.
-The set of properties will be passed to a discovery service where the service
tries to match the properties to a `CatalogFactory` and initiate a
corresponding catalog instance.
+In order to use custom catalogs with Flink SQL, users should implement a
corresponding catalog factory by implementing the `CatalogFactory` interface.
+This factory must provide a required "type" property which is matched against
the factory identifier during discovery.
Review comment:
The `type` property is rather DDL. How about:
```
The provided factory identifier will be used for matching against the
required `type` property in a SQL `CREATE CATALOG` DDL statement.
```
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java
##########
@@ -19,22 +19,103 @@
package org.apache.flink.table.factories;
import org.apache.flink.annotation.PublicEvolving;
+import org.apache.flink.configuration.ConfigOption;
+import org.apache.flink.configuration.ReadableConfig;
import org.apache.flink.table.catalog.Catalog;
+import org.apache.flink.table.catalog.exceptions.CatalogException;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
* A factory to create configured catalog instances based on string-based
properties. See also
- * {@link TableFactory} for more information.
+ * {@link Factory} for more information.
+ *
+ * <p>Note that this interface supports the {@link TableFactory} stack for
compatibility purposes.
+ * This is deprecated, however, and new implementations should implement the
{@link Factory} stack
+ * instead.
*/
@PublicEvolving
-public interface CatalogFactory extends TableFactory {
+public interface CatalogFactory extends TableFactory, Factory {
/**
* Creates and configures a {@link Catalog} using the given properties.
*
* @param properties normalized properties describing an external catalog.
* @return the configured catalog.
+ * @deprecated Use {@link this#createCatalog(Context)} instead and
implement {@link Factory}
+ * instead of {@link TableFactory}.
+ */
+ @Deprecated
+ default Catalog createCatalog(String name, Map<String, String> properties)
{
+ throw new CatalogException("Catalog factories must implement
createCatalog()");
+ }
+
+ /**
+ * Creates and configures a {@link Catalog} using the given context.
+ *
+ * <p>An implementation should perform validation and the discovery of
further (nested)
+ * factories in this method.
*/
- Catalog createCatalog(String name, Map<String, String> properties);
+ default Catalog createCatalog(Context context) {
+ throw new CatalogException("Catalog factories must implement
createCatalog(Context)");
+ }
+
+ /** Context provided when a catalog is created. */
+ interface Context {
+ /** Returns the name with which the catalog is created. */
+ String getName();
+
+ /**
+ * Returns the properties with which the catalog is created.
+ *
+ * <p>An implementation should perform validation of these properties.
+ */
+ Map<String, String> getProperties();
+
+ /** Gives read-only access to the configuration of the current
session. */
+ ReadableConfig getConfiguration();
+
+ /**
+ * Returns the class loader of the current session.
+ *
+ * <p>The class loader is in particular useful for discovering further
(nested) factories.
+ */
+ ClassLoader getClassLoader();
+ }
+
+ default Map<String, String> requiredContext() {
Review comment:
shall we split all legacy methods in a dedicated lower section,
otherwise this class is a bit messy with all these default methods.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
