David Rowley <dgrowleyml@gmail.com> writes:
> On Tue, 29 Aug 2023 at 07:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The bigger picture here is: what is the use-case for grouping by a
>> constant at all? Assuming that it is an error seems like a good
>> foolproofing restriction. The reason we felt we could keep the
>> "group by N" SQL92-ism after SQL99 redefined GROUP BY arguments is
>> exactly that there's no obvious use-case for grouping by a constant.
>> As soon as you allow it, "group by N" becomes hopelessly ambiguous.
> The new behaviour feels a bit inconsistent to me as it stands today.
> I can't write GROUP BY true, but I can write GROUP BY 1=1, which gets
> it beyond the parser and allows constant folding to turn it into GROUP
> BY true, which I couldn't specify because the parser would complain.
Sure, you can write any constant expression, for instance NULL::bool
would work. The question is where do we draw the line between SQL92
and SQL99 behaviors. I think "an undecorated constant is SQL92, and
therefore it must be an integer matching an output column number" is
a nice simple rule. The fact that TRUE and FALSE were not previously
treated as undecorated constants is an unintentional wart of the old
implementation, not something we ought to preserve.
> I had a look on dbfiddle and I see that MySQL 8.0 and SQLlite all
> allow GROUP BY true.
What do they do with GROUP BY 1, or GROUP BY 10000, or GROUP BY 1.0 ?
> I think if we used to, and those other databases
> do, then we might want to reconsider supporting it again, especially
> so now that someone has complained. I'm assuming it's just as simple
> as the attached patch, but I'm happy to listen if I've underestimated
> the complexity.
Sure, changing the behavior is trivial. Whether we *should* change
the behavior, and if so to what, is less so. I'm not really on
board with "we need to maintain bug-compatibility with an old
implementation artifact".
BTW, I poked around and couldn't find anything explaining this
fine point in the SGML docs, although the comments in
findTargetlistEntrySQL92 are clear about it. If we do anything
at all here, I think that ought to include documenting the behavior
more clearly (and I'm curious to see how you'd propose to explain
the behavior you want to users).
regards, tom lane
