You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "luoyuxia (via GitHub)" <gi...@apache.org> on 2023/04/26 11:34:25 UTC

[GitHub] [flink] luoyuxia commented on a diff in pull request #22166: [FLINK-31409][hive] Hive dialect should use public interfaces

luoyuxia commented on code in PR #22166:
URL: https://github.com/apache/flink/pull/22166#discussion_r1177746826


##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/HiveParserDMLHelper.java:
##########
@@ -331,20 +333,48 @@ public Operation createInsertOperation(HiveParserCalcitePlanner analyzer, RelNod
                 createInsertOperationInfo(
                         queryRelNode,
                         destTable,
-                        HiveParserBaseSemanticAnalyzer.parseCompoundName(catalogManager, tableName),
+                        HiveParserBaseSemanticAnalyzer.parseCompoundName(
+                                catalogRegistry, tableName),
                         staticPartSpec,
                         analyzer.getDestSchemaForClause(insClauseName),
                         overwrite);
 
         return new SinkModifyOperation(
-                catalogManager.getTableOrError(insertOperationInfo.f0),
+                getContextResolvedTable(insertOperationInfo.f0),
                 insertOperationInfo.f1,
                 insertOperationInfo.f2,
                 null, // targetColumns
                 insertOperationInfo.f3,
                 Collections.emptyMap());
     }
 
+    private ContextResolvedTable getContextResolvedTable(ObjectIdentifier tableIdentifier) {

Review Comment:
   Yes, we still needs to depend on `ContextResolvedTable` for it's need to construct `SinkModifyOperation` which is also `@Internal`. In fact, Hive dialect still use many internal operations. Maybe we need to expose them to public then,  but it may be another topic. 
   Considering our plan is decoupling Hive with flink-table-planner in the first, I think we can keep them to see how much these internal  interfaces/classes will affect the maintainance/development of Hive dialect to take further steps.
   Btw, it make me be aware of the title of the pr/jira is not correct,  it should be `Hive dialect shouldn't use interfaces in table-planner to build LogicPlan`.
   



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

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org