Re: Compatible defaults for LEAD/LAG

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Vik Fearing <vik(at)postgresfriends(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Compatible defaults for LEAD/LAG
Date: 2020-07-23 11:29:42
Message-ID: 6149A5FF-A923-4E1B-96F7-521411CA5C73@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 13 Jul 2020, at 19:23, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

> ne 31. 5. 2020 v 22:02 odesílatel Vik Fearing <vik(at)postgresfriends(dot)org <mailto:vik(at)postgresfriends(dot)org>> napsal:
> On 5/31/20 9:53 PM, Tom Lane wrote:
> > Vik Fearing <vik(at)postgresfriends(dot)org <mailto:vik(at)postgresfriends(dot)org>> writes:
> >> postgres=# SELECT LAG(n, 1, -99) OVER (ORDER BY n)
> >> postgres-# FROM (VALUES (1.1), (2.2), (3.3)) AS v (n)
> >> postgres-# ORDER BY n;
> >> ERROR: function lag(numeric, integer, integer) does not exist
> >> LINE 1: SELECT LAG(n, 1, -99) OVER (ORDER BY n)
> >> ^
> >
> > Yeah, we have similar issues elsewhere.
> >
> >> Attached is a patch that fixes this issue using the new anycompatible
> >> pseudotype. I am hoping this can be slipped into 13 even though it
> >> requires a catversion bump after BETA1.
> >
> > When the anycompatible patch went in, I thought for a little bit about
> > trying to use it with existing built-in functions, but didn't have the
> > time to investigate the issue in detail. I'm not in favor of hacking
> > things one-function-at-a-time here; we should look through the whole
> > library and see what we've got.
> >
> > The main thing that makes this perhaps-not-trivial is that an
> > anycompatible-ified function will match more cases than it would have
> > before, possibly causing conflicts if the function or operator name
> > is overloaded. We'd have to look at such cases and decide what we
> > want to do --- one answer would be to drop some of the alternatives
> > and rely on the parser to add casts, but that might slow things down.
> >
> > Anyway, I agree that this is a good direction to pursue, but not in
> > a last-minute-hack-for-v13 way.
>
> Fair enough. I put it in the commitfest app for version 14.
> https://commitfest.postgresql.org/28/2574/ <https://commitfest.postgresql.org/28/2574/>
> --
> Vik Fearing
>
> The patch is ok. It is almost trivial. It solves one issue, but maybe it introduces a new issue.
>
> Other databases try to coerce default constant expression to value expression. I found a description about this behaviour for MSSQL, Sybase, BigQuery.
>
> I didn't find related documentation for Oracle, and I have not a access to some running instance of Oracle to test it.
>
> Ansi SQL say - type of default expression should be compatible with lag expression, and declared type of result should be type of value expression.
>
> IWD 9075-2:201?(E) 6.10 <window function> - j) v)
>
> Current implementation is limited (and the behaviour is not user friendly in all details), but new behaviour (implemented by patch) is in half cases out of standard :(.
>
> These cases are probably not often - and they are really corner cases, but I cannot to qualify how much important this fact is.
>
> For users, the implemented feature is better, and it is safe. Implementation is trivial - is significantly simpler than implementation that is 100% standard, although it should not be a hard problem too (in analyze stage it probably needs a few lines too).
>
> There has to be a decision, because now we can go in both directions. After choosing there will not be a way back.

This patch is marked waiting for author, but it's not clear from this review
what we're waiting on. Should this be moved to Needs review or Ready for
Committer instead to solicit the input from a committer? ISTM the latter is
more suitable. Or did I misunderstand your review?

cheers ./daniel

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2020-07-23 11:35:24 Re: explain HashAggregate to report bucket and memory stats
Previous Message Amit Kapila 2020-07-23 11:00:43 Re: Parallel Seq Scan vs kernel read ahead