b-slim commented on a change in pull request #1384:
URL: https://github.com/apache/samza/pull/1384#discussion_r444319271
##########
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);
+ try {
+ RelRoot optimizedRelRoot =
+ RelRoot.of(getPlanner().transform(0, relTraitSet,
relRoot.project()), SqlKind.SELECT);
+ LOG.info("query plan with optimization:\n"
+ + RelOptUtil.toString(optimizedRelRoot.rel,
SqlExplainLevel.EXPPLAN_ATTRIBUTES));
+ return optimizedRelRoot;
+ } catch (Exception e) {
+ String errorMsg =
+ "Error while optimizing query plan:\n" +
RelOptUtil.toString(relRoot.rel, SqlExplainLevel.EXPPLAN_ATTRIBUTES);
+ LOG.error(errorMsg, e);
+ throw new SamzaException(errorMsg, e);
+ }
+ }
+
+ public RelRoot plan(String query) {
+ try {
+ Planner planner = getPlanner();
SqlNode sql = planner.parse(query);
SqlNode validatedSql = planner.validate(sql);
RelRoot relRoot = planner.rel(validatedSql);
- LOG.info("query plan:\n" + RelOptUtil.toString(relRoot.rel,
SqlExplainLevel.ALL_ATTRIBUTES));
- return relRoot;
+ LOG.info("query plan without optimization:\n"
+ + RelOptUtil.toString(relRoot.rel,
SqlExplainLevel.EXPPLAN_ATTRIBUTES));
Review comment:
If the main concern is that planning will fail with the new rules my
suggestion is to have it on by default and catch the exception and re-plan
without optimization. In this way we can learn the logs. It is up to you if you
think this can be too much work
----------------------------------------------------------------
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]
