Re: Compatible defaults for LEAD/LAG - Mailing list pgsql-hackers
From | Pavel Stehule |
---|---|
Subject | Re: Compatible defaults for LEAD/LAG |
Date | |
Msg-id | CAFj8pRDf1BJ7a7hGe43s-KKN9-3jOSpsgoHgRNQXXcqicZnKSQ@mail.gmail.com Whole thread Raw |
In response to | Re: Compatible defaults for LEAD/LAG (Daniel Gustafsson <daniel@yesql.se>) |
Responses |
Re: Compatible defaults for LEAD/LAG
|
List | pgsql-hackers |
čt 23. 7. 2020 v 13:29 odesílatel Daniel Gustafsson <daniel@yesql.se> napsal:
> On 13 Jul 2020, at 19:23, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> ne 31. 5. 2020 v 22:02 odesílatel Vik Fearing <vik@postgresfriends.org <mailto:vik@postgresfriends.org>> napsal:
> On 5/31/20 9:53 PM, Tom Lane wrote:
> > Vik Fearing <vik@postgresfriends.org <mailto:vik@postgresfriends.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?
Unfortunately, I don't know - I am not sure about committing this patch, and I am not able to qualify for impact.
This is nice example of usage of anycompatible type (that is consistent with other things in Postgres), but standard says something else.
It can be easily solved with https://commitfest.postgresql.org/28/2081/ - but Tom doesn't like this patch.
I am more inclined to think so this feature should be implemented differently - there is no strong reason to go against standard or against the implementations of other databases (and increase the costs of porting). Now the implementation is limited, but allowed behaviour is 100% ANSI.
On second hand, the implemented feature (behaviour) is pretty small, and really corner case, so maybe a simple solution (although it isn't perfect) is good.
So I would listen to the opinions of other people.
Regards
Pavel
cheers ./daniel
pgsql-hackers by date: