Re: Monotonic WindowFunc support for ntile(), percent_rank() and cume_dist()

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Monotonic WindowFunc support for ntile(), percent_rank() and cume_dist()
Date: 2023-01-24 13:38:58
Message-ID: 20230124133858.vw6bi2sot2cbljyh@liskov
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 24, 2023 at 02:00:33PM +1300, David Rowley wrote:
> Thanks for having a look at this.
>
> On Tue, 24 Jan 2023 at 13:26, Melanie Plageman
> <melanieplageman(at)gmail(dot)com> wrote:
>
> > Since all three cases are exactly the same code, maybe you could
> > macro-ize it and add a single comment?
>
> Hmm, I kinda like that it's being spelt out explicitly. To me, it
> seems clean and easy to read. I know we could have fewer lines of code
> with something else, but for me, being able to quickly see what the
> properties of the WindowFunc are without having to look at some other
> function is more important than saving some space in windowfuncs.c
>
> I'd likely feel differently if the code in question didn't all fit on
> my screen at once, but it does and I can see at a quick glance that
> the function is unconditionally monotonically increasing. Functions
> such as COUNT(*) are conditionally monotonically
> increasing/decreasing, depending on the frame options.
>
> If you feel strongly about that, then feel free to show me what you
> have in mind in more detail so I can think harder about it.

Nah, I don't feel strongly. I think it was because looking at the patch
in isolation, the repetition stands out but in the context of the rest
of the code it doesn't.

- Melanie

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2023-01-24 13:42:56 Re: to_hex() for negative inputs
Previous Message David Geier 2023-01-24 13:35:45 Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?