b-slim commented on a change in pull request #1384:
URL: https://github.com/apache/samza/pull/1384#discussion_r440323413
##########
File path:
samza-sql/src/main/java/org/apache/samza/sql/planner/QueryPlanner.java
##########
@@ -140,16 +171,46 @@ public RelRoot plan(String query) {
.operatorTable(new ChainedSqlOperatorTable(sqlOperatorTables))
.sqlToRelConverterConfig(SqlToRelConverter.Config.DEFAULT)
.traitDefs(traitDefs)
- .context(Contexts.EMPTY_CONTEXT)
- .costFactory(null)
+ .programs(Programs.hep(rules, true,
DefaultRelMetadataProvider.INSTANCE))
.build();
- Planner planner = Frameworks.getPlanner(frameworkConfig);
+ planner = Frameworks.getPlanner(frameworkConfig);
+ return planner;
+ } catch (Exception e) {
+ String errorMsg = "Failed to create planner.";
+ LOG.error(errorMsg, e);
+ throw new SamzaException(errorMsg, e);
+ }
+ }
+ private RelRoot optimize(RelRoot relRoot) {
+ RelTraitSet relTraitSet = RelTraitSet.createEmpty();
+ relTraitSet = relTraitSet.plus(EnumerableConvention.INSTANCE);
Review comment:
This is basically tricking the planner to use Enum Convention plus
Volcano planner to generate a Plan with Convention of
`EnumerableConvention.INSTANCE`. This will lead to complex branches down the
line when doing the conversion as a follow up stage. You don't think the best
way to do this is by using Calcite Conventions where Rules and Translation is
happening at the same time to avoid complex code and very cryptic comments on
what to expect when doing the translation ?
----------------------------------------------------------------
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]
