[
https://issues.apache.org/jira/browse/CALCITE-6143?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Steven Talbot updated CALCITE-6143:
-----------------------------------
Description:
{code:java}
// pseudo-code repro case
b.scan("table")
.project(b.field(0), b.field(1), b.field(2))
.aggregate(b.groupKey(ImmutableBitSet.of(1)), b.aggregateCall(SUM,
b.field(2)), b.aggregatecall(SUM, b.field(2))){code}
^^ easier to see the change that introduced the bug in some ways than think
about a repro, basically
[https://github.com/apache/calcite/commit/de4631f62cc06b22199c1c14b687ea8a06ea06ec#diff-3be99bddc7edf13dc8198da7425d7e97c97237e3561c263fd74903b4a42d8cd9R2434]
refactored "groupSet" to a 'final' var, so introduced a 'groupSet2' to handle
permutations to it, but missed the last use at the bottom of the method for
dedupAggregate. So if we get to that point,
[https://github.com/apache/calcite/blame/379f41d3be465992328d5659ea62b8355e0399d1/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L2511]
the aggregate gets re-built with the original groupSet, ignoring any
permutation that happened to groupSet2 in the aggregate pruning.
Obviously one can workaround by turning one of these configs off, but they're
both on by default so this is a bit of a landmine in Calcite 1.35, since it's
unlikely to be caught by a test until someone writes a specific sort of query
in production.
was:
{code:java}
// pseudo-code repro case
b.scan("table")
.project(b.field(0), b.field(1), b.field(2))
.aggregate(b.groupKey(ImmutableBitSet.of(1)), b.aggregateCall(SUM,
b.field(2)), b.aggregatecall(SUM, b.field(2))){code}
^^ easier to see the change that introduced the bug in some ways than think
about a repro, basically
[https://github.com/apache/calcite/commit/de4631f62cc06b22199c1c14b687ea8a06ea06ec#diff-3be99bddc7edf13dc8198da7425d7e97c97237e3561c263fd74903b4a42d8cd9R2434]
refactored "groupSet" to a 'final' var, but missed the last use at the bottom
of the method for dedupAggregate. So if we get to that point,
[https://github.com/apache/calcite/blame/379f41d3be465992328d5659ea62b8355e0399d1/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L2511]
the aggregate gets re-built with the original groupSet, ignoring any
permutation that happened to groupSet2 in the aggregate pruning.
Obviously one can workaround by turning one of these configs off, but they're
both on by default so this is a bit of a landmine in Calcite 1.35, since it's
unlikely to be caught by a test until someone writes a specific sort of query
in production.
> RelBuilder.aggregate pruneInputOfAggregate+dedupAggregateCalls blocks can be
> wrong if both trigger
> --------------------------------------------------------------------------------------------------
>
> Key: CALCITE-6143
> URL: https://issues.apache.org/jira/browse/CALCITE-6143
> Project: Calcite
> Issue Type: Bug
> Reporter: Steven Talbot
> Priority: Major
>
> {code:java}
> // pseudo-code repro case
> b.scan("table")
> .project(b.field(0), b.field(1), b.field(2))
> .aggregate(b.groupKey(ImmutableBitSet.of(1)), b.aggregateCall(SUM,
> b.field(2)), b.aggregatecall(SUM, b.field(2))){code}
> ^^ easier to see the change that introduced the bug in some ways than think
> about a repro, basically
> [https://github.com/apache/calcite/commit/de4631f62cc06b22199c1c14b687ea8a06ea06ec#diff-3be99bddc7edf13dc8198da7425d7e97c97237e3561c263fd74903b4a42d8cd9R2434]
> refactored "groupSet" to a 'final' var, so introduced a 'groupSet2' to
> handle permutations to it, but missed the last use at the bottom of the
> method for dedupAggregate. So if we get to that point,
> [https://github.com/apache/calcite/blame/379f41d3be465992328d5659ea62b8355e0399d1/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L2511]
> the aggregate gets re-built with the original groupSet, ignoring any
> permutation that happened to groupSet2 in the aggregate pruning.
> Obviously one can workaround by turning one of these configs off, but they're
> both on by default so this is a bit of a landmine in Calcite 1.35, since it's
> unlikely to be caught by a test until someone writes a specific sort of query
> in production.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
