Re: Compatible defaults for LEAD/LAG

čt 23. 7. 2020 v 13:29 odesílatel Daniel Gustafsson <daniel(at)yesql(dot)se>
napsal:

> > 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?
>

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
>
>