KurtYoung commented on a change in pull request #9981: 
[FLINK-13195][sql-client] Add create table support for SqlClient
URL: https://github.com/apache/flink/pull/9981#discussion_r344091764
 
 

 ##########
 File path: 
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/ExecutionContext.java
 ##########
 @@ -204,13 +208,8 @@ public ClusterID getClusterId() {
                return 
clusterClientFactory.createClusterDescriptor(executorConfig);
        }
 
-       public EnvironmentInstance createEnvironmentInstance() {
-               try {
-                       return wrapClassLoader(EnvironmentInstance::new);
-               } catch (Throwable t) {
-                       // catch everything such that a wrong environment does 
not affect invocations
-                       throw new SqlExecutionException("Could not create 
environment instance.", t);
-               }
+       public EnvironmentInstance getEnvironmentInstance() {
 
 Review comment:
   Actually, I'm not sure whether this simple change is the right way to go. 
   
   IIUC you are trying to make `LocalExecutor` stateful, right? I think we 
might need to first change the design of current SQL CLI, get the relations 
between `LocalExecutor`, `SessionContext`, `ExecutionContext` and 
`EnvironmentInstance` clear. 
   
   I've create a jira to track this issue: 
https://issues.apache.org/jira/browse/FLINK-14672
   
   I'd prefer to finish that issue first and then come back to this one. 

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