Postgres Pro
Downloads
Home   >   mailing lists

Thread: [PoC] Implementation of distinct in Window Aggregates: take two

[PoC] Implementation of distinct in Window Aggregates: take two

From
Ankit Pandey
Date:
Hi,


This is a PoC patch which implements distinct operation in window aggregates (without order by and for single column aggregation, final version may vary wrt these limitations). Purpose of this PoC is to get feedback on the approach used and corresponding implementation, any nitpicking as deemed reasonable.

Distinct operation is mirrored from implementation in nodeAgg. Existing partitioning logic determines if row is in partition and when distinct is required, all tuples for the aggregate column are stored in tuplesort. When finalize_windowaggregate gets called, tuples are sorted and duplicates are removed, followed by calling the transition function on each tuple.
When distinct is not required, the above process is skipped and the transition function gets called directly and nothing gets inserted into tuplesort.
Note: For each partition, in tuplesort_begin and tuplesort_end is involved to rinse tuplesort, so at any time, max tuples in tuplesort is equal to tuples in a particular partition.

I have verified it for interger and interval column aggregates (to rule out obvious issues related to data types).

Sample cases:

create table mytable(id int, name text);
insert into mytable values(1, 'A');
insert into mytable values(1, 'A');
insert into mytable values(5, 'B');
insert into mytable values(3, 'A');
insert into mytable values(1, 'A');

select avg(distinct id) over (partition by name) from mytable;
        avg        
--------------------
2.0000000000000000
2.0000000000000000
2.0000000000000000
2.0000000000000000
5.0000000000000000

select avg(id) over (partition by name) from mytable;
        avg        
--------------------
 1.5000000000000000
 1.5000000000000000
 1.5000000000000000
 1.5000000000000000
 5.0000000000000000

select avg(distinct id) over () from mytable;
        avg        
--------------------
 3.0000000000000000
 3.0000000000000000
 3.0000000000000000
 3.0000000000000000
 3.0000000000000000

select avg(distinct id)  from mytable;
        avg        
--------------------
 3.0000000000000000

This is my first-time contribution. Please let me know if anything can be
improved as I`m eager to learn.

Regards,
Ankit Kumar Pandey
Attachment

Re: [PoC] Implementation of distinct in Window Aggregates: take two

From
Shlok Kyal
Date:
Hi,

I went through the Cfbot, and some of the test cases are failing for
this patch. It seems like some tests are crashing:

https://api.cirrus-ci.com/v1/artifact/task/6291153444667392/crashlog/crashlog-postgres.exe_03b0_2023-11-07_10-41-39-624.txt

[10:46:56.546] Summary of Failures:
[10:46:56.546]
[10:46:56.546] 87/270 postgresql:postgres_fdw / postgres_fdw/regress
ERROR 11.10s exit status 1
[10:46:56.546] 82/270 postgresql:regress / regress/regress ERROR
248.55s exit status 1
[10:46:56.546] 99/270 postgresql:recovery /
recovery/027_stream_regress ERROR 161.40s exit status 29
[10:46:56.546] 98/270 postgresql:pg_upgrade /
pg_upgrade/002_pg_upgrade ERROR 253.31s exit status 29

link of tests failing:
https://cirrus-ci.com/task/6642997165555712
https://cirrus-ci.com/task/4602303584403456
https://cirrus-ci.com/task/5728203491246080
https://cirrus-ci.com/task/5165253537824768?logs=test_world#L511
https://cirrus-ci.com/task/6291153444667392

Thanks
Shlok Kumar Kyal

Re: [PoC] Implementation of distinct in Window Aggregates: take two

From
vignesh C
Date:
On Wed, 8 Nov 2023 at 11:46, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> Hi,
>
> I went through the Cfbot, and some of the test cases are failing for
> this patch. It seems like some tests are crashing:
>
https://api.cirrus-ci.com/v1/artifact/task/6291153444667392/crashlog/crashlog-postgres.exe_03b0_2023-11-07_10-41-39-624.txt
>
> [10:46:56.546] Summary of Failures:
> [10:46:56.546]
> [10:46:56.546] 87/270 postgresql:postgres_fdw / postgres_fdw/regress
> ERROR 11.10s exit status 1
> [10:46:56.546] 82/270 postgresql:regress / regress/regress ERROR
> 248.55s exit status 1
> [10:46:56.546] 99/270 postgresql:recovery /
> recovery/027_stream_regress ERROR 161.40s exit status 29
> [10:46:56.546] 98/270 postgresql:pg_upgrade /
> pg_upgrade/002_pg_upgrade ERROR 253.31s exit status 29
>
> link of tests failing:
> https://cirrus-ci.com/task/6642997165555712
> https://cirrus-ci.com/task/4602303584403456
> https://cirrus-ci.com/task/5728203491246080
> https://cirrus-ci.com/task/5165253537824768?logs=test_world#L511
> https://cirrus-ci.com/task/6291153444667392

The patch which you submitted has been awaiting your attention for
quite some time now.  As such, we have moved it to "Returned with
Feedback" and removed it from the reviewing queue. Depending on
timing, this may be reversible.  Kindly address the feedback you have
received, and resubmit the patch to the next CommitFest.

Regards,
Vignesh