From: | Mark Dilger <hornschnorter(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: "SELECT ... FROM DUAL" is not quite as silly as it appears |
Date: | 2019-01-27 20:04:52 |
Message-ID: | 810DE61F-7332-492E-A730-E78006388BB8@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On Jan 25, 2019, at 5:09 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> David Rowley <david(dot)rowley(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Mark Dilger | 2019-01-27 20:53:22 | Re: "SELECT ... FROM DUAL" is not quite as silly as it appears |
Previous Message | Mark Dilger | 2019-01-27 19:55:11 | Re: "SELECT ... FROM DUAL" is not quite as silly as it appears |