Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21109 )

Change subject: IMPALA-12872: Use Calcite for ...
......................................................................


Patch Set 4:

(54 comments)

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java@88
PS4, Line 88:    * location for the column will remain null.
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java@103
PS4, Line 103:       int position = (slotDesc.getColumn().getPosition() + 
nonPartitionedCols) % totalCols;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java@40
PS4, Line 40:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java@50
PS4, Line 50:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@67
PS4, Line 67: public class CalciteTable extends RelOptAbstractTable implements 
Table, Prepare.PreparingTable {
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@117
PS4, Line 117:       SlotRef slotref = new 
SlotRef(Path.createRawPath(baseTblRef.getUniqueAlias(), fieldName));
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@122
PS4, Line 122:             + "is a complex type (array/map/struct) column. This 
is not currently supported."));
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@181
PS4, Line 181:   @Override
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@182
PS4, Line 182:   public boolean columnHasDefaultValue(RelDataType rowType, int 
ordinal, InitializerContext initializerContext) {
line too long (113 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@186
PS4, Line 186:   @Override
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java@46
PS4, Line 46:     return stmtTableCache;
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@53
PS4, Line 53:  * to walk through all the steps of compiling the query (e.g. 
parsing, validating, etc... )
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@58
PS4, Line 58:   protected static final Logger LOG = 
LoggerFactory.getLogger(CalciteJniFrontend.class.getName());
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@96
PS4, Line 96:       
RelMetadataQuery.THREAD_PROVIDERS.set(JaninoRelMetadataProvider.of(DefaultRelMetadataProvider.INSTANCE));
line too long (111 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@105
PS4, Line 105:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@120
PS4, Line 120:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@205
PS4, Line 205:   public byte[] runThroughOriginalPlanner(byte[] 
thriftQueryContext) throws ImpalaException {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@105
PS4, Line 105:   /**
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@127
PS4, Line 127:       FeCatalog catalog, Set<TableName> tableNames)
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@145
PS4, Line 145:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@170
PS4, Line 170:    * tableNames
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@175
PS4, Line 175:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java@37
PS4, Line 37:  * CalcitePhysPlanCreator. This class is responsible for turning 
an ImpalaPlanRel Calcite plan
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java@47
PS4, Line 47:   private static final RelOptTable.ViewExpander NOOP_EXPANDER = 
(type, query, schema, path) -> null;
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java@52
PS4, Line 52:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java@39
PS4, Line 39:   protected static final Logger LOG = 
LoggerFactory.getLogger(CalciteValidator.class.getName());
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@77
PS4, Line 77:   protected static final Logger LOG = 
LoggerFactory.getLogger(ExecRequestCreator2.class.getName());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@91
PS4, Line 91:     TExecRequest request = 
createExecRequest(nodeWithExprs.planNode_, queryCtx.getTQueryCtx(),
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@102
PS4, Line 102:   public TExecRequest createExecRequest(PlanNode planNodeRoot, 
TQueryCtx queryCtx, PlannerContext plannerContext,
line too long (113 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@103
PS4, Line 103:       Analyzer analyzer, List<Expr> outputExprs, 
Collection<FeTable> tables) throws ImpalaException {
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@132
PS4, Line 132:     // to mimic the original planner behavior, use EXTENDED mode 
explain except for EXPLAIN statements
line too long (102 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@134
PS4, Line 134:     // TExplainLevel explainLevel = isExplain ? 
plannerContext.getQueryOptions().getExplain_level() :
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@142
PS4, Line 142:     String explainString2 = getExplainString(allFragments, 
TExplainLevel.STANDARD, plannerContext);
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@147
PS4, Line 147:     
queryCtx.setDesc_tbl_serialized(plannerContext.getRootAnalyzer().getDescTbl().toSerializedThrift());
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@155
PS4, Line 155:   List<PlanFragment> createPlans(PlanNode planNodeRoot, Analyzer 
analyzer, PlannerContext ctx,
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@173
PS4, Line 173:       List<PlanFragment> fragments = 
createPlanFragments(planNodeRoot, ctx, analyzer, outputExprs);
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@178
PS4, Line 178:         // The rootFragmentList contains the 'root' fragments of 
each of the parallel plans
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@188
PS4, Line 188:    * Create one or more plan fragments corresponding to the 
supplied single node physical plan.
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@195
PS4, Line 195:    * @return list of plan fragments in the order [root fragment, 
child of root ... leaf fragment]
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@199
PS4, Line 199:   private List<PlanFragment> createPlanFragments(PlanNode 
planNodeRoot, PlannerContext ctx, Analyzer analyzer,
line too long (110 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@211
PS4, Line 211:       // Create distributed plan. For insert/CTAS without limit, 
isPartitioned should be true.
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@213
PS4, Line 213:       // final boolean isPartitioned = stmtType_ == 
TStmtType.DML && !planNodeRoot.hasLimit();
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@242
PS4, Line 242:   private TExecRequest createExecRequest(TQueryCtx queryCtx, 
PlanFragment planFragmentRoot,
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@272
PS4, Line 272:   private String getExplainString(List<PlanFragment> fragments, 
TExplainLevel explainLevel, PlannerContext ctx) {
line too long (113 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@331
PS4, Line 331:   private static boolean invertJoins(PlanNode root, boolean 
isLocalPlan, Analyzer analyzer) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java@337
PS4, Line 337:       for (PlanNode child: root.getChildren()) inverted |= 
invertJoins(child, isLocalPlan, analyzer);
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@51
PS4, Line 51:  * the function signatures, all precisions are allowed, so this 
type is used when describing
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@53
PS4, Line 53:  * - types with precisions.  These also use Types (also of type 
ScalarType), but the precision
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@55
PS4, Line 55:  *
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@57
PS4, Line 57:  * - Normalized RelDataTypes.  While theoretically we should have 
been able to use SqlTypeName to
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@58
PS4, Line 58:  * have the same purpose as the Impala default dataypes, there is 
no SqlTypeName.STRING. Therefore,
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@68
PS4, Line 68:     RexBuilder rexBuilder = new RexBuilder(new 
JavaTypeFactoryImpl(new ImpalaTypeSystemImpl()));
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java
File 
java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java:

http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java@36
PS4, Line 36:   protected static final Logger LOG = 
LoggerFactory.getLogger(ImpalaTypeSystemImpl.class.getName());
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java@41
PS4, Line 41:   private static final int 
MAX_TIMESTAMP_WITH_LOCAL_TIME_ZONE_PRECISION = 15; // Up to nanos
line too long (92 > 90)



--
To view, visit http://gerrit.cloudera.org:8080/21109
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98
Gerrit-Change-Number: 21109
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Mon, 11 Mar 2024 19:38:16 +0000
Gerrit-HasComments: Yes

Reply via email to