Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options

From: Krasiyan Andreev <krasiyan(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org, david(at)fetter(dot)org, Oliver Ford <ojford(at)gmail(dot)com>
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
Date: 2020-04-30 19:50:00
Message-ID: CAN1PwonAnC-KkRyY+DtRmxQ8rjdJw+gcOsHruLr6EnF7zSMH=Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thank you very much for feedback and yes, that is very useful SQL syntax.
Maybe you miss my previous answer, but you are right, that patch is
currently dead,
because some important design questions must be discussed here, before
patch rewriting.

I have dropped support of from first/last for nth_value(),
but also I reimplemented it in a different way,
by using negative number for the position argument, to be able to get the
same frame in exact reverse order.
After that patch becomes much more simple and some concerns about
precedence hack has gone.

I have not renamed special bool type "ignorenulls"
(I know that it is not acceptable way for calling extra version
of window functions, but also it makes things very easy and it can reuse
frames),
but I removed the other special bool type "fromlast".

Attached file was for PostgreSQL 13 (master git branch, last commit fest),
everything was working and patch was at the time in very good shape, all
tests was passed.

I read previous review and suggestions from Tom about special bool type and
unreserved keywords and also,
that IGNORE NULLS could be implemented inside the WinGetFuncArgXXX
functions,
but I am not sure how exactly to proceed (some example will be very
helpful).

На чт, 30.04.2020 г. в 21:58 Stephen Frost <sfrost(at)snowman(dot)net> написа:

> Greetings,
>
> This seems to have died out, and that's pretty unfortunate because this
> is awfully useful SQL standard syntax that people look for and wish we
> had.
>
> * Andrew Gierth (andrew(at)tao11(dot)riddles(dot)org(dot)uk) wrote:
> > So I've tried to rough out a decision tree for the various options on
> > how this might be implemented (discarding the "use precedence hacks"
> > option). Opinions? Additions?
> >
> > (formatted for emacs outline-mode)
> >
> > * 1. use lexical lookahead
> >
> > +: relatively straightforward parser changes
> > +: no new reserved words
> > +: has the option of working extensibly with all functions
> >
> > -: base_yylex needs extending to 3 lookahead tokens
>
> This sounds awful grotty and challenging to do and get right, and the
> alternative (just reserving these, as the spec does) doesn't seem so
> draconian as to be that much of an issue.
>
> > * 2. reserve nth_value etc. as functions
> >
> > +: follows the spec reasonably well
> > +: less of a hack than extending base_yylex
> >
> > -: new reserved words
> > -: more parser rules
> > -: not extensible
> >
>
> For my 2c, at least, reserving these strikes me as entirely reasonable.
> Yes, it sucks that we have to partially-reserve some additional
> keywords, but such is life. I get that we'll throw syntax errors
> sometimes when we might have given a better error, but I think we can
> accept that.
>
> > (now goto 1.2.1)
>
> Hmm, not sure this was right? but sure, I'll try...
>
> > *** 1.2.1. Check the function name in parse analysis against a fixed
> list.
> >
> > +: simple
> > -: not extensible
>
> Seems like this is more-or-less required since we'd be reserving them..?
>
> > *** 1.2.2. Provide some option in CREATE FUNCTION
> >
> > +: extensible
> > -: fairly intrusive, adding stuff to create function and pg_proc
>
> How would this work though, if we reserve the functions as keywords..?
> Maybe I'm not entirely following, but wouldn't attempts to use other
> functions end up with syntax errors in at least some of the cases,
> meaning that having other functions support this wouldn't really work?
> I don't particularly like the idea that some built-in functions would
> always work but others would work but only some of the time.
>
> > *** 1.2.3. Do something magical with function argument types
> >
> > +: doesn't need changes in create function / pg_proc
> > -: it's an ugly hack
>
> Not really a fan of 'ugly hack'.
>
> > * 3. "just say no" to the spec
> >
> > e.g. add new functions like lead_ignore_nulls(), or add extra boolean
> > args to lead() etc. telling them to skip nulls
> >
> > +: simple
> > -: doesn't conform to spec
> > -: using extra args isn't quite the right semantics
>
> Ugh, no thank you.
>
> Thanks!
>
> Stephen
>

Attachment Content-Type Size
pg13_ignore_nulls.patch text/x-patch 51.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2020-04-30 19:52:22 Re: design for parallel backup
Previous Message Alvaro Herrera 2020-04-30 19:39:49 Re: Avoiding hash join batch explosions with extreme skew and weird stats