Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options - Mailing list pgsql-hackers
| From | Krasiyan Andreev |
|---|---|
| Subject | Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options |
| Date | |
| Msg-id | CAN1PwonAnC-KkRyY+DtRmxQ8rjdJw+gcOsHruLr6EnF7zSMH=Q@mail.gmail.com Whole thread Raw |
| In response to | Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options (Stephen Frost <sfrost@snowman.net>) |
| List | 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).
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@snowman.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@tao11.riddles.org.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
pgsql-hackers by date: