I think at this point `BeamQueryPlanner` needs refactor. Basically it does
multiple things and now even more than it did, plus violates encapsulation:
* creates a config;
* creates an instance of a planner using the config;
* exposes a bunch of implementation details on how it constructs the planner;
* exposes an interface to parse the query and convert it to RelNodes;
To me these look like this can be split into a bunch of independent small
modules with fewer responsibilities and some inversion of control (each is
simpler, easier to read and test):
```
class BeamCalciteFrameworkConfig {
static FrameworkConfig create(CalciteConnection connection) { ... }
}
class CalcitePlannerFactory {
Planner createCalcitePlanner(
FrameworkConfig calciteFrameworkConfig,
BeamSqlPipelineOptions pipelineOptions) {
return hasPlannerOption(pipelineOptions))
? createCustomPlanner(calciteFrameworkConfig)
: Frameworks.getPlanner(calciteFrameworkConfig);
}
}
}
```
Rename BeamQueryPlanner to something like `QueryParser`
```
class QueryParser {
static create(Planner calcitePlanner) {
this.planner = calcitePlanner;
}
}
```
Then just configure everything within `BeamSqlEnv`:
```
class BeamSqlEnv {
private BeamSqlEnv(...) {
...
calciteFrameworkConfig = BeamCalciteFrameworkConfig.create(connection);
calcitePlannerImpl = CalcitePlannerFactory.create(calciteFrameworkConfig,
getPipelineOptions());
queryParser = QueryParser.create(calcitePlannerImpl);
}
}
```
[ Full content available at: https://github.com/apache/beam/pull/6598 ]
This message was relayed via gitbox.apache.org for [email protected]
