You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/01/21 16:48:11 UTC

[GitHub] [flink] twalthr commented on a change in pull request #10917: [FLINK-15612][table] Refactor DataTypeLookup to DataTypeFactory

twalthr commented on a change in pull request #10917: [FLINK-15612][table] Refactor DataTypeLookup to DataTypeFactory
URL: https://github.com/apache/flink/pull/10917#discussion_r369118164
 
 

 ##########
 File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/CatalogManager.java
 ##########
 @@ -68,19 +72,88 @@
 	// The name of the built-in catalog
 	private final String builtInCatalogName;
 
-	public CatalogManager(String defaultCatalogName, Catalog defaultCatalog) {
+	private final DataTypeFactory typeFactory;
+
+	private CatalogManager(
+			ClassLoader classLoader,
+			ReadableConfig config,
+			String defaultCatalogName,
+			Catalog defaultCatalog,
+			@Nullable ExecutionConfig executionConfig) {
+		checkNotNull(classLoader, "Class loader cannot be null");
+		checkNotNull(config, "Config cannot be null");
 		checkArgument(
 			!StringUtils.isNullOrWhitespaceOnly(defaultCatalogName),
 			"Default catalog name cannot be null or empty");
 		checkNotNull(defaultCatalog, "Default catalog cannot be null");
+
 		catalogs = new LinkedHashMap<>();
 		catalogs.put(defaultCatalogName, defaultCatalog);
-		this.currentCatalogName = defaultCatalogName;
-		this.currentDatabaseName = defaultCatalog.getDefaultDatabase();
+		currentCatalogName = defaultCatalogName;
+		currentDatabaseName = defaultCatalog.getDefaultDatabase();
 
-		this.temporaryTables = new HashMap<>();
+		temporaryTables = new HashMap<>();
 		// right now the default catalog is always the built-in one
-		this.builtInCatalogName = defaultCatalogName;
+		builtInCatalogName = defaultCatalogName;
+
+		typeFactory = new DataTypeFactoryImpl(classLoader, config, executionConfig);
 
 Review comment:
   This is already prepared for looking up types in the future which requires access to the catalog manager itself. But we can also postpone this until we really need it.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services