pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Mark Dilger <hornschnorter@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
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: Sun, 27 Jan 2019 12:04:52 -0800
Message-ID: <810DE61F-7332-492E-A730-E78006388BB8@gmail.com> (raw)
In-Reply-To: <20496.1548464946@sss.pgh.pa.us>
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>



> On Jan 25, 2019, at 5:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> As far as I can see the patch is ready to go, but I'll defer to Mark,
>> who's also listed on the reviewer list for this patch.
> 
> Mark, are you planning to do further review on this patch?
> I'd like to move it along, since (IMO anyway) we need it in
> before progress can be made on
> https://commitfest.postgresql.org/21/1664/
> 
> 			regards, tom lane


These two observations are not based on any deep understanding of
your patch, but just some cursory review:

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.

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.


Applying your patch and running the regression tests is failing
left and right, so I'm working to pull a fresh copy from git and
build again -- probably just something wrong in my working directory.

mark






view thread (33+ messages)  latest in thread

Message-ID: <810DE61F-7332-492E-A730-E78006388BB8@gmail.com>
Permalink:  ../810DE61F-7332-492E-A730-E78006388BB8@gmail.com/
Also on:    postgresql.org/message-id/810DE61F-7332-492E-A730-E78006388BB8@gmail.com

 · 

reply

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: hornschnorter@gmail.com, tgl@sss.pgh.pa.us, 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: <810DE61F-7332-492E-A730-E78006388BB8@gmail.com>

* 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