On 13 December 2013 01:14, Tom Lane <tgl@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
