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

 ##########
 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:
   How about we pass the `DataTypeFactory` instead of `config` & 
`executionConfig`. It would make it easier to possibly mock the 
`DataTypeFactory`.
   
   We can construct this default implementation in the builder.

----------------------------------------------------------------
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]


With regards,
Apache Git Services

Reply via email to