dawidwys commented on a change in pull request #10878: [FLINK-15599][table] SQL
client requires both legacy and blink planner to be on the classpath
URL: https://github.com/apache/flink/pull/10878#discussion_r368004811
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/ExecutionContext.java
##########
@@ -269,12 +272,11 @@ public Pipeline createPipeline(String name,
Configuration flinkConfig) {
if (streamExecEnv != null) {
// special case for Blink planner to apply batch
optimizations
// note: it also modifies the ExecutionConfig!
- if (executor instanceof ExecutorBase) {
+ if (isBlinkPlanner) {
Review comment:
I am not sure if I would go for that solution. I don't necessarily like the
way we set this property. It also complicates the instantiation logic and
couples it very tightly with the internal structure.
How about we perform the check in a following manner:
```
if (isBlinkPlanner(executor.getClass)) {
...
}
...
public boolean isBlinkPlanner(Class<? extends Executor> executorClass) {
try {
return ExecutorBase.class.isAssignableFrom(executorClass);
} catch (NoClassDefFound ex) {
// blink planner might not be on the class path
return false;
}
}
```
----------------------------------------------------------------
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
