[
https://issues.apache.org/jira/browse/CALCITE-7687?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101408#comment-18101408
]
Etienne Pelissier commented on CALCITE-7687:
--------------------------------------------
Test:
{code:java}
// Per-column null fractions, distinct so the value returned names the column
// that the predicate resolved to.
private static final double[] NULL_FRACTION = {0.13, 0.42, 0.77}; // a, b, c
// Reports the null fraction of the column the predicate names, else Calcite's
// generic guess -- what a real column-statistics handler would do.
private static @Nullable Double nullFractionOf(@Nullable RexNode predicate) {
if (predicate instanceof RexCall
&& predicate.getKind() == SqlKind.IS_NULL
&& ((RexCall) predicate).getOperands().get(0) instanceof RexInputRef) {
final RexInputRef ref =
(RexInputRef) ((RexCall) predicate).getOperands().get(0);
return NULL_FRACTION[ref.getIndex()];
}
return RelMdUtil.guessSelectivity(predicate);
}
private static RelDataType abcRowType(RelDataTypeFactory typeFactory) {
final RelDataType varchar =
typeFactory.createTypeWithNullability(
typeFactory.createSqlType(SqlTypeName.VARCHAR), true);
return typeFactory.builder()
.add("a", varchar).add("b", varchar).add("c", varchar).build();
}
// Found by stock getSelectivity(TableScan, ...) through RelOptTable#unwrap,
// the hook added by CALCITE-4223. No custom RelMdSelectivity is involved.
private static class SelectivityByColumnTable extends AbstractTable
implements BuiltInMetadata.Selectivity.Handler {
@Nullable RexNode received;
@Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
return abcRowType(typeFactory);
}
@Override public @Nullable Double getSelectivity(RelNode r,
RelMetadataQuery mq, @Nullable RexNode predicate) {
received = predicate;
return nullFractionOf(predicate);
}
}
// Aggregate(group={1, 2}, COUNT($0)) over a scan of (a, b, c). RelBuilder
// yields the non-prefix group set directly, so no rule is needed; the same
// shape arises from "SELECT count(a) FROM t GROUP BY b, c HAVING c IS NULL"
// once AggregateProjectMergeRule has fired.
private static RelNode aggregateGroupingOnFields1And2(AbstractTable table) {
final SchemaPlus root = Frameworks.createRootSchema(true);
root.add("T", table);
final FrameworkConfig config =
Frameworks.newConfigBuilder().defaultSchema(root).build();
final RelBuilder b = RelBuilder.create(config);
return b.scan("T")
.aggregate(b.groupKey(1, 2), b.count(false, "cnt", b.field(0)))
.build();
}
private static RexNode isNullOn(RelNode rel, int i) {
final RexBuilder rexBuilder = rel.getCluster().getRexBuilder();
return rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL,
rexBuilder.makeInputRef(rel.getRowType().getFieldList().get(i).getType(), i));
}
// Aggregate output field i is input field groupSet.nth(i), so a predicate on
// a group key must be converted before it is pushed to the input.
@Test void testSelectivityAggregateConvertsPredicateToInputFields() {
final SelectivityByColumnTable table = new SelectivityByColumnTable();
final RelNode agg = aggregateGroupingOnFields1And2(table);
final RelMetadataQuery mq = agg.getCluster().getMetadataQuery();
// Aggregate output $1 is column "c", input $2.
assertThat(mq.getSelectivity(agg, isNullOn(agg, 1)),
isAlmost(NULL_FRACTION[2]));
assertThat(table.received, hasToString("IS NULL($2)"));
// Aggregate output $0 is column "b", input $1.
assertThat(mq.getSelectivity(agg, isNullOn(agg, 0)),
isAlmost(NULL_FRACTION[1]));
assertThat(table.received, hasToString("IS NULL($1)"));
}
// An aggregate call has no equivalent expression on the input, so a predicate
// on one must not be pushed. Pushability is tested against groupSet = {1, 2},
// which holds input indices, so output $2 slips through;
range(getGroupCount())
// = {0, 1} would refuse it.
@Test void testSelectivityAggregateDoesNotPushAggregateCallPredicate() {
final SelectivityByColumnTable table = new SelectivityByColumnTable();
final RelNode agg = aggregateGroupingOnFields1And2(table);
final RelMetadataQuery mq = agg.getCluster().getMetadataQuery();
// Aggregate output $2 is COUNT($0), not a group key.
assertThat(mq.getSelectivity(agg, isNullOn(agg, 2)),
isAlmost(DEFAULT_SELECTIVITY));
assertThat(table.received, nullValue());
}
{code}
Fails with:
{noformat}
java.lang.AssertionError:
Expected: a numeric value within <1.0E-5> of <0.77>
but: <0.42> differed by <0.34999> more than delta <1.0E-5>
at
org.apache.calcite.test.RelMetadataTest.testSelectivityAggregateConvertsPredicateToInputFields(RelMetadataTest.java:1857)
java.lang.AssertionError:
Expected: a numeric value within <1.0E-5> of <0.25>
but: <0.77> differed by <0.5199900000000001> more than delta <1.0E-5>
at
org.apache.calcite.test.RelMetadataTest.testSelectivityAggregateDoesNotPushAggregateCallPredicate(RelMetadataTest.java:1879)
{noformat}
> RelMdSelectivity and RelMdDistinctRowCount for Aggregate can propagate a
> predicate with wrong references
> --------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-7687
> URL: https://issues.apache.org/jira/browse/CALCITE-7687
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.42.0
> Reporter: Etienne Pelissier
> Priority: Major
> Labels: pull-request-available
>
> This is the same defect as CALCITE-4414, but in the {{Aggregate}} overloads,
> which were not swept [when that issue was
> fixed|https://github.com/apache/calcite/commit/b4e399cb35224d8c8d55f02b7cf2b9649a3b28a4]
> for {{Calc}} in 1.27.0.
> An {{Aggregate}} derives its row type as {{{}(group keys..., agg
> calls...){}}}, so output field {{i}} is input field {{{}groupSet.nth(i){}}}.
> Two metadata handlers forward a predicate expressed over the aggregate's
> *output* to the aggregate's *input* without applying that translation.
>
> *Minimal repros covering both handlers are in the comments of this ticket.*
> h3. 1. [RelMdSelectivity#getSelectivity(Aggregate,
> ...)|https://github.com/apache/calcite/blob/7939fa2163467205726764fb2e575f8b289c1b8b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdSelectivity.java#L178]
> {code:java}
> RelOptUtil.splitFilters(rel.getGroupSet(), predicate, pushable, notPushable);
> RexNode childPred = RexUtil.composeConjunction(rexBuilder, pushable, true);
> // childPred not translated
> Double selectivity = mq.getSelectivity(rel.getInput(), childPred);{code}
> Two independent problems.
> *(a) No translation.* Exactly as in
> [CALCITE-4414|https://github.com/apache/calcite/commit/b4e399cb35224d8c8d55f02b7cf2b9649a3b28a4].
> Compare {{{}getSelectivity(Project, ...){}}}, which calls
> {{RelOptUtil.pushPastProject}} before recursing, and {{{}getSelectivity(Calc,
> ...){}}}, which calls {{RelOptUtil.pushPastCalc}} since
> [CALCITE-4414|https://github.com/apache/calcite/commit/b4e399cb35224d8c8d55f02b7cf2b9649a3b28a4].
> *(b) Wrong pushability* *bitmap.* {{predicate}} is in output index space, but
> {{rel.getGroupSet() }}holds *input* indices, so {{splitFilters}} compares the
> two spaces against one another. The correct bitmap is
> {{{}ImmutableBitSet.range(rel.getGroupCount()){}}}, which is what
> {{RelMdDistinctRowCount}} already uses for the same purpose, so the two
> handlers currently disagree.
> h3. 2. [RelMdDistinctRowCount#getDistinctRowCount(Aggregate,
> ...)|https://github.com/apache/calcite/blob/7939fa2163467205726764fb2e575f8b289c1b8b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdDistinctRowCount.java#L168]
> {code:java}
> RelOptUtil.splitFilters(
> ImmutableBitSet.range(rel.getGroupCount()), predicate, pushable, notPushable);
> RexNode childPreds = RexUtil.composeConjunction(rexBuilder, pushable, true);
> // set the bits as they correspond to the child input
> RelMdUtil.setAggChildKeys(groupKey, rel, childKey);
> // childPreds not translated
> Double distinctRowCount = mq.getDistinctRowCount(rel.getInput(),
> childKey.build(), childPreds);{code}
> One problem.
> *(a) No translation.*
> h3. 3. Symptom
> Unlike CALCITE-4414, which threw {{{}ArrayIndexOutOfBoundsException{}}}, this
> is silent.
> {{splitFilters}} only pushes conjuncts whose refs are inside the bitmap, so
> the pushed index is always valid: it just names a different column.
>
> That is harmless while the handler below keys only off {{SqlKind
> (RelMdUtil.guessSelectivity)}} , which is why it has gone unnoticed.
> It becomes a *wrong estimate* for any table supplying a
> {{BuiltInMetadata.Selectivity.Handler}} through {{{}RelOptTable.unwrap{}}}.
> h3. 4. Suggested fix
> [{{FlinkRelMdUtil.splitPredicateOnAgg}}|https://github.com/apache/flink/blob/12197ea92a5667073bc0c6810e526a39979d835c/flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/utils/FlinkRelMdUtil.scala#L556]
> already addresses both problems:
>
> {code:java}
> RelOptUtil.splitFilters(ImmutableBitSet.range(0, numOfGroupKey), predicate,
> pushable, notPushable)
> val adjustments = new Array[Int](aggOutputFields.size)
> grouping.zipWithIndex.foreach { case (bit, index) => adjustments(index) = bit
> - index }
> pushCondition.accept(new RelOptUtil.RexInputConverter(
> rexBuilder, aggOutputFields, aggInputFields, adjustments)){code}
> {{RexInputConverter}} is already used this way by both {{Union}} handlers. A
> {{RelOptUtil.pushPastAggregate}} helper alongside {{pushPastProject}} /
> {{pushPastCalc}} would let both handlers share one implementation.
> h3. 5. Downstream impact
> *Drill* - most exposed:
> [{{DrillRelMdSelectivity#getScanSelectivity}}|https://github.com/apache/drill/blob/23bc6619705fe4f625a4dbe68e0044bd8dead73b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdSelectivity.java#L117]
> consults per-column statistics, so this is a wrong estimate rather than a
> differently-wrong constant. Neither Drill handler overrides {{{}Aggregate{}}}.
> *Hive* - neither handler overrides {{{}Aggregate{}}}.
> *Kylin* - {{{}DefaultRelMetadataProvider{}}}, no custom handlers.
> *Flink* - unaffected (fix above).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
