flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-10768) Move external catalog related code from TableEnvironment to CatalogManager
Date Tue, 06 Nov 2018 00:34:00 GMT

    [ 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:

> 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 &amp; 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

View raw message