Re: ruleutils vs. empty targetlists

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ruleutils vs. empty targetlists
Date: 2013-12-13 09:36:00
Message-ID: CAEZATCWENAvwOrJ7+Yj4_gNzy2fj5QGL0tS+w+oL-fKAJSFPyw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 13 December 2013 01:14, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2013-12-13 09:53:02 Minor comment improvement
Previous Message Pavel Stehule 2013-12-13 09:09:02 Re: patch: make_timestamp function