From: Tom Lane <tgl@sss.pgh.pa.us>
To: Oliver Ford <ojford@gmail.com>
cc: Erik Rijkers <er@xs4all.nl>, Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Add RANGE with values and exclusions clauses to the Window
 Functions
In-reply-to: 
 <CAGMVOduaHeA8q+kvCqX-UfuV_=jHvb-XK+1KJEiNy80Drk=y_A@mail.gmail.com>
References: 
 <CAGMVOdu9sivPAxbNN0X+q19Sfv9edEPv=HibOJhB14TJv_RCQg@mail.gmail.com>
 <b2374b9461c349b17b50c77f3a70e1a1@xs4all.nl>
 <CAGMVOdsopg5ZAuJh9+ceq-s0pU2qqZsZn-jbL4A1c=h1iWaZVA@mail.gmail.com>
 <CAGMVOdvrZCwJg0BngM9GMO+LK=j20vhSfdk1gTTBNNH311Vgsw@mail.gmail.com>
 <bc8a1d397fa81d6ae33c297d0224184b@xs4all.nl>
 <2c084c1c2bf599ceb2afa87874de8bc1@xs4all.nl>
 <CAGMVOdvETRCKpeT06Uoq5RsNUOdH7d1iYy7C1Pze=L5=gBzs-Q@mail.gmail.com>
 <cdbe62a6eaddae00fa770c09ed346fc0@xs4all.nl>
 <CAGMVOdttRSUU3=tjgdhr902HmUVL1GLQWG0ZTVXi7OY59Xo0Xw@mail.gmail.com>
 <13279.1515535159@sss.pgh.pa.us>
 <CAGMVOdtYSNJusckQ_9vBkMsZBQzan=Uw-Z-uYG2fD9rZ2TP7pA@mail.gmail.com>
 <4139.1515567978@sss.pgh.pa.us>
 <CAGMVOdvJMfOzm0v4psCoAaVGfCbdFwNKzaSEhfTj8rE9UCz-eQ@mail.gmail.com>
 <c30e6457a6de09d26425af997f71996e@xs4all.nl>
 <CAGMVOdtGe=30u6D2WqwvWchE+-yNb9jKnEeG8BHyw7jHiGPR7w@mail.gmail.com>
 <27788.1517069693@sss.pgh.pa.us>
 <CAGMVOduaHeA8q+kvCqX-UfuV_=jHvb-XK+1KJEiNy80Drk=y_A@mail.gmail.com>
Comments: In-reply-to Oliver Ford <ojford@gmail.com>
	message dated "Mon, 29 Jan 2018 16:55:59 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <10567.1517261334.1@sss.pgh.pa.us>
Date: Mon, 29 Jan 2018 16:28:54 -0500
Message-ID: <10568.1517261334@sss.pgh.pa.us>
Precedence: bulk

Oliver Ford <ojford@gmail.com> writes:
> [ 0001-window-frame-v9.patch ]

I've started to go through this in some detail, and I'm wondering why
you invented a FRAMEOPTION_EXCLUDE_NO_OTHERS option bit rather than
just representing that choice as default (0).  As you have it, a
window definition that explicitly specifies EXCLUDE NO OTHERS will be
considered unequal to one that just defaults to that behavior, in
e.g. transformWindowFuncCall().  That seems undesirable, if not
outright wrong.  Against that, having the bit allows ruleutils.c
to print "EXCLUDE NO OTHERS" when the input included that, but that
seems like a wash if not an anti-feature.  We've never been big on
making ruleutils output distinguish explicit from implicit selection
of a default setting, and in this case it could possibly lead to
outputting a query in a form that is not backwards-compatible to
older PG versions, when there's no need to be incompatible.

If there's some other consideration I'm missing, please say what;
otherwise I'll change it.

BTW, I generally recommend not including a catversion change in
submitted patches.  It causes merge problems any time some other
catversion-bumping patch gets committed, and it can't possibly be
the right value for the final commit since you aren't likely to
be able to predict that date in advance.  It surely doesn't hurt
to remind the committer that a catversion bump is needed, but just
do that in the submission message.

			regards, tom lane