Two allocations in visit(VirtualTableScan) scale with the table for no gain.
A fresh empty LogicalValues is built inside the per-row loop, although emptyRowType and emptyRowValue are hoisted above it and identical for every row:
|
|
|
RelDataType emptyRowType = typeFactory.createStructType(List.of(), List.of()); |
|
ImmutableList<ImmutableList<RexLiteral>> emptyRowValue = ImmutableList.of(ImmutableList.of()); |
|
|
|
List<RelNode> projects = new ArrayList<>(); |
|
for (final List<RexNode> rexRow : convertedRows) { |
|
RelNode values = LogicalValues.create(relBuilder.getCluster(), emptyRowType, emptyRowValue); |
|
RelNode project = |
|
LogicalProject.create( |
|
values, Collections.emptyList(), rexRow, rowType, Collections.emptySet()); |
|
projects.add(project); |
|
} |
An N-row table therefore gets N identical single-empty-tuple nodes, each computing its own digest. Calcite RelNodes are immutable and may have several parents, so one instance above the loop serves every projection.
And both value lists are filled for every value of every row, while the branch below reads exactly one of them and discards the other:
|
List<List<RexNode>> convertedRows = new ArrayList<>(); |
|
// The same values with the cast a nullable literal converts as taken off, which is the form a |
|
// LogicalValues tuple would hold them in -- where they are literals at all. |
|
List<List<RexNode>> tupleValues = new ArrayList<>(); |
|
for (final Expression.NestedStruct rowExpr : virtualTableScan.getRows()) { |
|
List<RexNode> convertedRow = new ArrayList<>(); |
|
List<RexNode> tupleRow = new ArrayList<>(); |
|
for (int column = 0; column < rowExpr.fields().size(); column++) { |
|
Expression field = rowExpr.fields().get(column); |
|
RelDataType declaredType = rowType.getFieldList().get(column).getType(); |
|
RexNode value = |
|
valueAsDeclared(field.accept(expressionRexConverter, context), declaredType); |
|
convertedRow.add(value); |
|
// Only a converted literal has its nullability cast taken off. A cast the plan carries |
|
// itself is doing work -- it has a failure behavior, and it is part of what round-trips -- |
|
// so a row holding one is computed rather than tabulated. |
|
tupleRow.add(field instanceof Expression.Literal ? unwrapNullabilityCast(value) : value); |
|
} |
|
convertedRows.add(convertedRow); |
|
tupleValues.add(tupleRow); |
|
} |
|
|
|
// A LogicalValues tuple holds nothing but literals, and whether a value is one is a property of |
|
// what it converts to rather than of the Substrait expression it came from: a struct converts |
|
// to a ROW call and a list to an array constructor, literal or not. Neither belongs in a tuple |
|
// anyway -- Calcite orders tuples by casting each value to Comparable, and the value of a row |
|
// literal is a list of RexLiterals, which are not. |
|
boolean encodableAsTuples = |
|
tupleValues.stream().flatMap(List::stream).allMatch(value -> value instanceof RexLiteral); |
For an R-by-C table that is R extra lists plus R times C extra slots held through the whole conversion, and unwrapNullabilityCast runs on every literal even for a table that ends up computed. Since encodableAsTuples needs the unwrapped form to decide, one list plus a running flag -- stop filling the tuple list at the first non-literal, unwrap in a second pass when the flag survives -- covers both branches.
Found while reviewing #1189.
Two allocations in
visit(VirtualTableScan)scale with the table for no gain.A fresh empty
LogicalValuesis built inside the per-row loop, althoughemptyRowTypeandemptyRowValueare hoisted above it and identical for every row:substrait-java/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java
Lines 888 to 899 in 934a60e
An N-row table therefore gets N identical single-empty-tuple nodes, each computing its own digest. Calcite
RelNodes are immutable and may have several parents, so one instance above the loop serves every projection.And both value lists are filled for every value of every row, while the branch below reads exactly one of them and discards the other:
substrait-java/isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java
Lines 830 to 858 in 934a60e
For an R-by-C table that is R extra lists plus R times C extra slots held through the whole conversion, and
unwrapNullabilityCastruns on every literal even for a table that ends up computed. SinceencodableAsTuplesneeds the unwrapped form to decide, one list plus a running flag -- stop filling the tuple list at the first non-literal, unwrap in a second pass when the flag survives -- covers both branches.Found while reviewing #1189.