From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)
Date: Mon, 12 Sep 2016 10:19:14 -0400
Message-ID: <24315.1473689954@sss.pgh.pa.us> (raw)
In-Reply-To: <20160827214829.zo2dfb5jaikii5nw@alap3.anarazel.de>
References: <20160523005327.v2tr7obytitxcnna@alap3.anarazel.de>
<20160822214023.aaxz5l4igypowyri@alap3.anarazel.de>
<20160827214829.zo2dfb5jaikii5nw@alap3.anarazel.de>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Andres Freund <andres@anarazel.de> writes:
> Attached is a significantly updated patch series (see the mail one up
> for details about what this is, I don't want to quote it in its
> entirety).
I've finally cleared my plate enough to start reviewing this patchset.
> 0001-Add-some-more-targetlist-srf-tests.patch
> Add some test.
I think you should go ahead and push this tests-adding patch now, as it
adds documentation of the current behavior that is a good thing to have
independently of what the rest of the patchset does. Also, I'm worried
that some of the GROUP BY tests might have machine-dependent results
(if they are implemented by hashing) so it would be good to get in a few
buildfarm cycles and let that settle out before we start changing the
answers.
I do have some trivial nitpicks about 0001:
# rules cannot run concurrently with any test that creates a view
-test: rules psql_crosstab amutils
+test: rules psql_crosstab amutils tsrf
Although tsrf.sql doesn't currently need to create any views, it doesn't
seem like a great idea to assume that it never will. Maybe add this
after misc_functions in the previous parallel group, instead?
+-- it's weird to GROUP BYs that increase the number of results
"it's weird to have ..."
+-- nonsensically that seems to be allowed
+UPDATE fewmore SET data = generate_series(4,9);
"nonsense that seems to be allowed..."
+-- SRFs are now allowed in RETURNING
+INSERT INTO fewmore VALUES(1) RETURNING generate_series(1,3);
s/now/not/, apparently
More to come later, but 0001 is pushable with these fixes.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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, andres@anarazel.de
Subject: Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)
In-Reply-To: <24315.1473689954@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