On 13 December 2013 01:14, Tom Lane <t...@sss.pgh.pa.us> wrote: > The only thing I've come across that arguably doesn't work is SELECT > DISTINCT: > > regression=# select distinct from pg_database; > -- > (8 rows) > > The reason this says "8 rows" is that the produced plan is just a seqscan > of pg_database returning no columns. The parser produced a Query with an > empty distinctClause, so the planner thinks no unique-ification is > required, so it doesn't produce any sort/uniq or hashagg node. This seems > a bit bogus to me; mathematically, it seems like what this ought to mean > is that all rows are equivalent and so you get just one empty row out. > However, I don't see any practical use-case for that behavior, so I'm > disinclined to spend time on inventing a solution --- and any solution > would probably require a change in Query representation, making it not > back-patchable anyway. Instead I suggest we add an error check in parse > analysis that throws an error for DISTINCT with no columns. If anyone is > ever motivated to make this case do something sane, they can remove that > check and fix up whatever needs fixing up. The attached first-draft patch > doesn't yet have such a prohibition, though. > > Another point is that there is a second use of target_list in the grammar, > which is "RETURNING target_list". I did not change that one into > "RETURNING opt_target_list", because if I had, it would have an issue > similar to DISTINCT's: RETURNING with no arguments would be represented > the same as no RETURNING at all, leading to probably not the behavior > the user expected. We've already got that problem if you say "RETURNING > zero_column_table.*", making me wonder if a parse-analysis-time error > for that case wouldn't be a good thing. > > Comments? >
I can't think of any practical uses for this kind of query, so I don't think it's worth worrying too much about its results until/unless someone comes up with a real use-case. However, given that we currently support queries like "select distinct * from nocols" (albeit with rather odd results), I don't think we should start throwing new errors for them. Perhaps the actual risk of a backwards-compatibility break is small, but so too is any benefit from adding such new errors. So +1 for the patch as-is, with no new errors. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
