> On 2013-08-30 06:34:47 -0700, David Johnston wrote: >> Tom Lane-2 wrote >> >> I was one who sent a bug report - this error is not too dangerous, but >> it >> >> is hidden, and difficult to find, if you don't know what can be >> happen. >> >> Same as bug with plpgsql and SQL identifier collisions. If you >> >> understand, >> >> then you can protect self well and simply. If not, then it is a magic >> >> error. So still I am thing so best solution is >> > >> >> a) a warning when detect ORDER BY in variadic aggregates >> > >> > Such a warning would never be tolerated by users, because it would >> appear >> > even when the query is perfectly correct. >> > >> >> b) disallow ORDER BY in variadic aggregates in classic syntax, and >> enable >> >> it only in WITHIN GROUP syntax where is safe , >> > >> > And we're *not* inventing randomly different syntax for variadic >> > aggregates. That ship sailed when we did it this way for regular >> > functions. >> >> In the example case the problem is that ORDER BY constant is a valid, if >> not-very-useful, construct. Can we warn on this specific usage and thus >> mitigate many of the potential avenues of mis-use? > > That doesn't help against something like »SELECT string_agg(somecol > ORDER BY bar, separator)« where separator is a column. > >> If we alter syntax for mitigation purposes I'd want to consider requiring >> parentheses around the columns that belong to the ORDER BY instead of >> using >> the full extended syntax of WITHIN GROUP. > > I think that ship has sailed. The syntax is there and it's not going > away. Requiring different syntaxes for variadic/nonvariadic usages is > going to be a way much bigger pitfall for users.
Neither suggestion (nor any suggestion I would imagine) is going to "solve" the problem. The goal is to minimize the size of the exposure.
For the second ORDER BY (col1, col2) suggestion it would be added and recommended so those using that syntax would have less to worry about. This would apply to ALL invocations, not just variadic.
you cannot use this syntax - probably, because (col1, col2) is same as ROW(col1, col2) and this syntax is supported now. So there is a collision. I had a same idea, but I don't think so it is possible
