[
https://issues.apache.org/jira/browse/FLINK-10768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16675924#comment-16675924
]
ASF GitHub Bot commented on FLINK-10768:
----------------------------------------
xuefuz commented on issue #7011: [FLINK-10768][Table & SQL] Move external
catalog related code from TableEnvironment to CatalogManager
URL: https://github.com/apache/flink/pull/7011#issuecomment-436088040
Thanks for the PR. I quickly went over the changes and have some high-level
comments:
1. It would be better if there is clearer distinction of responsibility for
the CatalogManager class. If it manages catalogs and temp tables/functions,
then it should just expose those as needed. If we need something extra, then we
need some justification.
2. Since we are encapsulating catalog, Calcite schema, and temp
tables/functions in CatalogManager, we should hide them as much as possible.
Exposing something like registerExternalCatalog() might trigger some questions.
3. Intuitively TableEnvironment depends on CatalogManager but not vise
versa. Thus, I'm not sure if and why we need a circular reference between them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Move external catalog related code from TableEnvironment to CatalogManager
> --------------------------------------------------------------------------
>
> Key: FLINK-10768
> URL: https://issues.apache.org/jira/browse/FLINK-10768
> Project: Flink
> Issue Type: Sub-task
> Components: Table API & SQL
> Reporter: Bowen Li
> Assignee: Bowen Li
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.8.0
>
>
> Add a new CatalogManager class and port existing Calcite-directly-related
> code from TableEnvironment into CatalogManager.
> Background: there are two parallel efforts going on right now - FLINK-10686,
> driven by Timo, includes moving external catalogs APIs from flink-table to
> flink-table-common, also from Scala to Java; FLINK-10744 I'm working on right
> now to integrate Flink with Hive and enhance external catalog functionality.
> As discussed with @twalthr in FLINK-10689, we'd better parallelize these
> efforts while introducing minimal overhead for integrating them later. Our
> agreed way is to writing new code/feature related to external catalogs/hive
> in Java in flink-table first then move to other module like
> flink-table-common, this way we can minimize migration efforts. If existing
> classes are modified for a feature we can start migrating them to Java in a
> separate commit first and then perform the actual feature changes, and
> migrated classes can be placed in flink-table/src/main/java until we find a
> better module structure.
> Thus, this is NOT a feature, but purely refactor, thus no new functions
> should be introduced. It acts the pre-requisite for FLINK-10698
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
