From: Tom Lane <tgl@sss.pgh.pa.us>
To: Mark Dilger <hornschnorter@gmail.com>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: "SELECT ... FROM DUAL" is not quite as silly as it appears
Date: Mon, 28 Jan 2019 18:04:29 -0500
Message-ID: <26005.1548716669@sss.pgh.pa.us> (raw)
In-Reply-To: <75650E87-F3B6-4CA3-88E9-86C21FE358E9@gmail.com>
References: <15944.1521127664@sss.pgh.pa.us>
<9565.1539573909@sss.pgh.pa.us>
<154342395130.2093.8013327151440360209.pgcf@coridan.postgresql.org>
<12357.1543518669@sss.pgh.pa.us>
<CAKJS1f-ZRNnb7-NxY1odPR=ZjEiJx=_094aQZsEvJ6JEnzDjKg@mail.gmail.com>
<20472.1546631337@sss.pgh.pa.us>
<CAKJS1f8OKaw4LV6rTf4gedO97fzgo-V6bFOoC-r4UK6Fdaex=Q@mail.gmail.com>
<21565.1547498912@sss.pgh.pa.us>
<CAKJS1f9RAWOscHa0Tboo4XhgaMqq_hHhXdapsxGV-Yru4mzr-g@mail.gmail.com>
<20496.1548464946@sss.pgh.pa.us>
<810DE61F-7332-492E-A730-E78006388BB8@gmail.com>
<75650E87-F3B6-4CA3-88E9-86C21FE358E9@gmail.com>
Mark Dilger <hornschnorter@gmail.com> writes:
>> The swtich in src/backend/parser/analyze.c circa line 2819 should
>> probably have an explicit case for RTE_RESULT rather than just a
>> comment and allowing the default to log "unrecognized RTE type",
>> since it's not really unrecognized, just unexpected. But I'm not
>> too exercised about that, and won't argue if you want to leave it
>> as is.
Meh --- I doubt we need two different "can't happen" messages there.
The reason I treated this differently from some other places is that
transformLockingClause is only used during parsing, when there certainly
shouldn't be any RTE_RESULT RTEs present. Some of the other functions
in that file are also called from outside the parser, so that it's
less certain they couldn't see a RTE_RESULT, so I added explicit
errors for them.
There's been some talk of having more uniform handling of switches
on enums, which might change the calculus here (i.e. we might not want
to have a default: case at all). But I don't feel a need to add code
to transformLockingClause till we have that.
>> Similarly, in src/backend/commands/explain.c, should there be a
>> case for T_Result in ExplainTargetRel's switch circa line 2974?
>> I'm not sure given your design whether this could ever be relevant,
>> but I noticed that T_Result / RTE_RELATION isn't handled here.
We don't get to that function for a T_Result plan (cf. switch in
ExplainNode), so it'd just be dead code.
> Ok, version 6 of the patch applies cleanly, compiles, and passes
> all tests for me on my platform (mac os x).
Again, thanks for reviewing!
Pushed now. I did yield to popular demand and change the temp table's
name in join.sql to be "onerow" instead of "dual" ;-)
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, hornschnorter@gmail.com, david.rowley@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
Subject: Re: "SELECT ... FROM DUAL" is not quite as silly as it appears
In-Reply-To: <26005.1548716669@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox