You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Taras Ledkov <tl...@gridgain.com> on 2021/10/04 17:30:50 UTC

Review request: CALCITE-4818 (AggregateExpandDistinctAggregatesRule must infer correct data type for top aggregate calls)

Hi,

Please review the patch for the issue CALCITE-4818 [1], see PR#2560 [2].

Looks like the rule 'AggregateExpandDistinctAggregatesRule' contains 
another bug with inferring result type of the top aggregate calls.

e.g.

If the type system expand sum type like postgress:
SUM(TINYINT | SMALLINT | INTEGER) ->  BIGINT
SUM(BIGINT) -> DECIMAL

The query:
SELECT SUM(i), SUM(DISTINCT i) FROM TBL;
where i - INTEGER field.

produces the plan:
LogicalProject(EXPR$0=[CAST($0):BIGINT], EXPR$1=[$1])
  LogicalAggregate(group=[{}], EXPR$0=[SUM($1)], EXPR$1=[SUM($0)]) -->  
RowType[DECIMAL, BIGINT]
     LogicalAggregate(group=[{0}], 
EXPR$0=[SUM($0)])                                 --> RowType[INTEGER, 
BIGINT]
       LogicalProject(COMM=[$6])
         LogicalTableScan(table=[[CATALOG, SALES, EMP]])

But the rule ignores the change row type in the bottom aggregate.

I propose the simple fix: pass 'null' type to the 'AggregateCall.create' 
call to infer aggregate type from the input.

[1]. https://issues.apache.org/jira/browse/CALCITE-4818
[2]. https://github.com/apache/calcite/pull/2560

-- 
Taras Ledkov
Mail-To: tledkov@gridgain.com


Re: Review request: CALCITE-4818 (AggregateExpandDistinctAggregatesRule must infer correct data type for top aggregate calls)

Posted by Taras Ledkov <tl...@gridgain.com>.
Hi Calcite team,

Gently reminder about review / merge the patch for  CALCITE-4818 [1]

[1]. https://issues.apache.org/jira/browse/CALCITE-4818

On 15.10.2021 15:16, Taras Ledkov wrote:
> Hi,
>
> Gently reminder about review.
>
> On 04.10.2021 20:30, Taras Ledkov wrote:
>> Hi,
>>
>> Please review the patch for the issue CALCITE-4818 [1], see PR#2560 [2].
>>
>> Looks like the rule 'AggregateExpandDistinctAggregatesRule' contains 
>> another bug with inferring result type of the top aggregate calls.
>>
>> e.g.
>>
>> If the type system expand sum type like postgress:
>> SUM(TINYINT | SMALLINT | INTEGER) ->  BIGINT
>> SUM(BIGINT) -> DECIMAL
>>
>> The query:
>> SELECT SUM(i), SUM(DISTINCT i) FROM TBL;
>> where i - INTEGER field.
>>
>> produces the plan:
>> LogicalProject(EXPR$0=[CAST($0):BIGINT], EXPR$1=[$1])
>>  LogicalAggregate(group=[{}], EXPR$0=[SUM($1)], EXPR$1=[SUM($0)]) 
>> -->  RowType[DECIMAL, BIGINT]
>>     LogicalAggregate(group=[{0}], 
>> EXPR$0=[SUM($0)])                                 --> 
>> RowType[INTEGER, BIGINT]
>>       LogicalProject(COMM=[$6])
>>         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
>>
>> But the rule ignores the change row type in the bottom aggregate.
>>
>> I propose the simple fix: pass 'null' type to the 
>> 'AggregateCall.create' call to infer aggregate type from the input.
>>
>> [1]. https://issues.apache.org/jira/browse/CALCITE-4818
>> [2]. https://github.com/apache/calcite/pull/2560
>>
-- 
Taras Ledkov
Mail-To: tledkov@gridgain.com


Re: Review request: CALCITE-4818 (AggregateExpandDistinctAggregatesRule must infer correct data type for top aggregate calls)

Posted by Taras Ledkov <tl...@gridgain.com>.
Hi,

Gently reminder about review.

On 04.10.2021 20:30, Taras Ledkov wrote:
> Hi,
>
> Please review the patch for the issue CALCITE-4818 [1], see PR#2560 [2].
>
> Looks like the rule 'AggregateExpandDistinctAggregatesRule' contains 
> another bug with inferring result type of the top aggregate calls.
>
> e.g.
>
> If the type system expand sum type like postgress:
> SUM(TINYINT | SMALLINT | INTEGER) ->  BIGINT
> SUM(BIGINT) -> DECIMAL
>
> The query:
> SELECT SUM(i), SUM(DISTINCT i) FROM TBL;
> where i - INTEGER field.
>
> produces the plan:
> LogicalProject(EXPR$0=[CAST($0):BIGINT], EXPR$1=[$1])
>  LogicalAggregate(group=[{}], EXPR$0=[SUM($1)], EXPR$1=[SUM($0)]) -->  
> RowType[DECIMAL, BIGINT]
>     LogicalAggregate(group=[{0}], 
> EXPR$0=[SUM($0)])                                 --> RowType[INTEGER, 
> BIGINT]
>       LogicalProject(COMM=[$6])
>         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
>
> But the rule ignores the change row type in the bottom aggregate.
>
> I propose the simple fix: pass 'null' type to the 
> 'AggregateCall.create' call to infer aggregate type from the input.
>
> [1]. https://issues.apache.org/jira/browse/CALCITE-4818
> [2]. https://github.com/apache/calcite/pull/2560
>
-- 
Taras Ledkov
Mail-To: tledkov@gridgain.com