dblink: add polymorphic functions.

On Fri, Feb 20, 2015 at 7:06 AM, Corey Huinker <corey.huinker@gmail.com> wrote:

Proposed patch attached.

At quick glance, this patch lacks two things:
- regression test coverage
- documentation
(Note: do not forget to add your name in the field "Author" when
adding a new patch in the CF app).
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Thanks - completely new to this process, so I'm going to need
walking-through of it. I promise to document what I learn and try to add
that to the commitfest wiki. Where can I go for guidance about
documentation format and regression tests?

Author field is presently being finicky, reported that to Magnus already.

On Thu, Feb 19, 2015 at 11:00 PM, Michael Paquier <michael.paquier@gmail.com

On Fri, Feb 20, 2015 at 2:50 PM, Corey Huinker <corey.huinker@gmail.com> wrote:

Thanks - completely new to this process, so I'm going to need
walking-through of it. I promise to document what I learn and try to add
that to the commitfest wiki. Where can I go for guidance about documentation
format and regression tests?

Here are some guidelines about how to submit a patch:
https://wiki.postgresql.org/wiki/Submitting_a_Patch

You can have as well a look here to see how extensions like dblink are
structured:
http://www.postgresql.org/docs/devel/static/extend-pgxs.html
What you need to do in the case of your patch is to add necessary test
cases to in sql/dblink.sql for the new functions you have added and
then update the output in expected/. Be sure to not break any existing
things as well. After running the tests in your development
environment output results are available in results then pg_regress
generates a differential file in regressions.diffs.

For the documentation, updating dblink.sgml with the new function
prototypes would be sufficient. With perhaps an example(s) to show
that what you want to add is useful.
Regards,
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I seem to be getting tripped up in the regression test. This line was found
in regression.diff

+ ERROR: could not stat file
"/home/ubuntu/src/postgres/contrib/dblink/tmp_check/install/usr/local/pgsql/share/extension/dblink--1.2.sql":
No such file or directory

The file dblink--1.2.sql does exist
in /home/ubuntu/src/postgres/contrib/dblink/

~/src/postgres/contrib/dblink$ ls -la dblink--1.?.sql
-rw-rw-r-- 1 ubuntu ubuntu 5.7K Feb 22 16:02 dblink--1.1.sql
-rw-rw-r-- 1 ubuntu ubuntu 6.8K Feb 22 16:50 dblink--1.2.sql

But it evidently isn't making it into the tmp_check dir.

What needs to happen for new files to be seen by the regression test
harness?

On Fri, Feb 20, 2015 at 1:14 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Mon, Feb 23, 2015 at 12:03 PM, Corey Huinker <corey.huinker@gmail.com> wrote:

+ ERROR: could not stat file
"/home/ubuntu/src/postgres/contrib/dblink/tmp_check/install/usr/local/pgsql/share/extension/dblink--1.2.sql":
No such file or directory

Didn't you forget to add dblink--1.2.sql to DATA in contrib/dblink
Makefile? That would explain why this file has not been included in
the temporary installation deployed by pg_regress.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Yes, that was it, I discovered it myself and should have posted a
"nevermind".

Now I'm slogging through figuring out where to find elog() messages from
the temporary server. It's slow, but it's progress.

On Sun, Feb 22, 2015 at 10:39 PM, Michael Paquier <michael.paquier@gmail.com

"nevermind". Found it.

On Sun, Feb 22, 2015 at 11:18 PM, Corey Huinker <corey.huinker@gmail.com>
wrote:

Changes in this patch:
- added polymorphic versions of dblink_fetch()
- upped dblink version # to 1.2 because of new functions
- migration 1.1 -> 1.2
- DocBook changes for dblink(), dblink_get_result(), dblink_fetch()

On Sun, Feb 22, 2015 at 11:38 PM, Corey Huinker <corey.huinker@gmail.com>
wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/22/2015 10:26 PM, Corey Huinker wrote:

Changes in this patch: - added polymorphic versions of
dblink_fetch() - upped dblink version # to 1.2 because of new
functions - migration 1.1 -> 1.2 - DocBook changes for dblink(),
dblink_get_result(), dblink_fetch()

The previous patch was missing dblink--1.1--1.2.sql and
dblink--1.2.sql. I have added them, so it should apply cleanly against
git master, but not done any actual review yet.

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVmYTEAAoJEDfy90M199hl4tcQAJVIDiJn/ogFlxPSOxIQ1XRF
hYllqLTCALIyfDWsq5oizVrs3uFF5TpqMrFxpfpLhKbeGgGpnaOhP5rISw3DD1NA
P73MDVbP0/+Q2pwAk174+teXxqFBK3gQi4wgtaq0bC4aTC+LlphYImDbb6ExfrRR
CFlEV4MoC3vFsOKRjGalcv/iaM7HIZSn8lilmynCFx96BDwTgrmZu5vSk17a5MsO
oOc1s+1eIiZ7JpUGcYHwCmunC2Aed8OtcLjCFu3BTKTJEq1xhkbvHPFZvLBI6CtD
CI74SIHdtTg56Rm8lsJFnkg8SM9QW8kEKP/eJedS/ft5d3dFdOwYfORh+2qwmsCo
JOvtriUEVs835HGTatuh47dscwgCt0d6SA0a7rp4UxmoQTmohyt5fN8LW2fJpHd8
bj7du7SUv6n3GwxZT+PBSALkvD011shNl0/AxehOoqXHmL86G+c3qc0vHEF5ulId
jeq/ECCX509Ef+DmjFoCtnotI/oXB74JmgOqy3RtvuuZ4hgsRzzT9kyKWmcpCHQ8
ZsmwmRY5xrjClzf7dsvh8LjyM1nOAoRNQhHLlj9I8lWKupChaWaY1PH0qzAdYm9p
a1kKTV4sfuNDqV57OUk/u33lvHVx1HDyvdCNRPHz+7cyw8Zka8jAGXvzq8vZp+bV
Grl29j/8fGaErff1CNZl
=Bh5j
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/05/2015 12:25 PM, Joe Conway wrote:

On 02/22/2015 10:26 PM, Corey Huinker wrote:

Changes in this patch: - added polymorphic versions of
dblink_fetch() - upped dblink version # to 1.2 because of new
functions - migration 1.1 -> 1.2 - DocBook changes for dblink(),
dblink_get_result(), dblink_fetch()

The previous patch was missing dblink--1.1--1.2.sql and
dblink--1.2.sql. I have added them, so it should apply cleanly
against git master, but not done any actual review yet.

Looking at the argument handling logic in dblink_fetch() and
dblink_record_internal(), it has been messy for a while, but this
patch serves to make it even worse.

I wonder if it isn't better to just loop through all the args with
get_fn_expr_argtype() every time and then test for the exact signature
match? Another alternative might be to create a wrapper C function for
each variant SQL function, but that seems like it could also get
messy, especially with dblink_record_internal() since we would have to
deal with every variant of all the external facing callers.

Thoughts?

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVmdM8AAoJEDfy90M199hldowQAJ8F14qSJC9BIGdxKwRIDCS2
bzbWoynYhGiDmvffmD3e3MAUrFwh+zS3QIN5BcJVBrdLnzjZIgHFz83Z1iuH1HVw
39z0sSZVJ0C7DvPV6UiNoDpKnyAJQUu3+A5ebWKj5AYOA0AVunA7J1vSMfXAYThV
zQ6lYEpCOFq0owIyjUFpjvXUSZFs6AHEQC5wQ/UW+EXCZKl0OYQtSf8oxi8m4DFu
xRrjM+bO7LmOrBPa5fPvQOXHr5KJRjq9x7CfU+a8mJaJh4r1MmDRp7iLVxlMae0k
YVdmsLP9FOS+RhAdmmKHsTWiEJIFhffKWqcahBXGdOOWjzUVzih/LAL0BS2z44AU
ygVW/ORg5ua7Y4zxg4PUKbIkvhA+qRs0WUpuY+nYZoYAayP3VqsWP3VvMYb/e/Fb
nyws+/C1wC2aIQxgoF/+Whdfh4eTcFMSK9Qsc0pdMleDizz9O0qauHWzglo89x8X
t+s2zNnmZ0NtWd4PSTcMAcq59v288CoKDME+gjT7A6jthEbBnkx6SdEjh/NLhJba
dKKv99ZP0Rg5v8pFpQ/3TzdBB1UifUz3nd6ubeWNPjAdBbiW1FFD+I6booPmaxh0
EOYLBVkVgJFgaz0bWI5P6bDi55LDQlUQEfLRE3dxoNdffivNNfElOkXly4VShB4i
YlCc/0A1opGZb3iJwo4X
=8VAQ
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, Jul 5, 2015 at 9:00 PM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/05/2015 12:25 PM, Joe Conway wrote:

On 02/22/2015 10:26 PM, Corey Huinker wrote:

Changes in this patch: - added polymorphic versions of
dblink_fetch() - upped dblink version # to 1.2 because of new
functions - migration 1.1 -> 1.2 - DocBook changes for dblink(),
dblink_get_result(), dblink_fetch()

The previous patch was missing dblink--1.1--1.2.sql and
dblink--1.2.sql. I have added them, so it should apply cleanly
against git master, but not done any actual review yet.

Looking at the argument handling logic in dblink_fetch() and
dblink_record_internal(), it has been messy for a while, but this
patch serves to make it even worse.

I wonder if it isn't better to just loop through all the args with
get_fn_expr_argtype() every time and then test for the exact signature
match? Another alternative might be to create a wrapper C function for
each variant SQL function, but that seems like it could also get
messy, especially with dblink_record_internal() since we would have to
deal with every variant of all the external facing callers.

Thoughts?

When I was digging into it, it seemed that the parameter sniffing was on
the verge of combinatorical chaos, which I assumed was due to parameter
checking being expensive.
And while there was no processing for the anyelement parameters, it did
throw off the parameter counts, thus the extra complexity.
If it's possible to suss out whether the last parameter is the polymorphic
type, then we could bump down the arg-count by one, and have the old messy
logic tree.

I also didn't delve into whether or not it was dangerous to ask
get_fn_expr_argtype() for parameters beyond PG_NARGS(). If it is, then we
would still have the complicated signature-testing tree, but at least we
could separate out the signature poking from the actual processing, and
that might add some clarity.

We could walk the arglist and handle each arg as you go, but that gets
complicated with the text params because their meaning is often determined
by whether the next param is also text. That has the potential to be every
bit as messy.

So from this, I see five options:

1. Leave it.
It's a mess, but we have test case coverage to show it works.

2. A bloody minded approach
if ((PG_NARGS() == 3) && (get_fn_expr_argtype(...,0) == TEXTOID) &&
(get_fn_expr_argtype(...,1) == TEXTOID) && (get_fn_expr_argtype(...,2) ==
BOOLOID))
{
}
else if ((PG_NARGS() == 3) && (get_fn_expr_argtype(...,0) == TEXTOID)
&& (get_fn_expr_argtype(...,1) == BOOLOID) && (get_fn_expr_argtype(...,2)
== WHATEVER_ANYELEMENTS_ARE_OID))
{
}
else if ...

Pro:
It's utterly clear which signature was found
Con:
A lot of param checks, though most will shortcircuit
The number of checks itself might become clutter.

3. Separate signature testing from using the parameters.
if (PG_NARGS() == 3)
{
if (get_fn_expr_argtype(fcinfo->flinfo, 2) == BOOLOID)
{
signature = TEXTTEXTBOOL
}
else if (get_fn_expr_argtype(fcinfo->flinfo, 1) == BOOLOID)
{
signature = TEXTBOOLANY
}
else
{
signature = TEXTTEXTANY
}
}

switch(signature)
{
case TEXTTEXTBOOL:
t1 = text_to_cstring(PG_GETARG_TEXT_PP(0);
t2 = text_to_cstring(PG_GETARG_TEXT_PP(1);
fail = PG_GETARG_BOOL(2);
break;

Pro:
Still getting to the signature with a minimum of comparisons.
Con:
A fair number of #defines for each signature combination (minus
anyelements, as we never have to process those).
Big(?) waterfall of options.
The mess has just been segregated, not eliminated.

4. Walk the arglist, assigning as you go, on the assumption that if the
current arg is text field it will be the last one encountered, and
re-assigning the pointers when we discover otherwise.

Pro:
A tight loop, probably the least amount of code.
I *think* this is what Joe was suggesting earlier.
Con:
We just made a mini-state engine, sorta like what you have to do
with event driven parsers (XML SAX, for example).

5. Walk the arglist like in #3, assigning bool/int parameters immediately,
but just count the text parameters, and assign them based on how many we
found.

Pro:
Same as #4, but my high school CS teacher won't come out of
retirement just to scold me for repurposing variables.
Con:
If at some point in the future we add more text parameters such
that simple ordering is ambiguous (i.e. were those text fields at positions
1,2,3 or 1,2,5?) this would break down.

I'm happy to try to code whichever of these people think is the most
promising.

On Mon, Jul 6, 2015 at 4:25 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/22/2015 10:26 PM, Corey Huinker wrote:

Changes in this patch: - added polymorphic versions of
dblink_fetch() - upped dblink version # to 1.2 because of new
functions - migration 1.1 -> 1.2 - DocBook changes for dblink(),
dblink_get_result(), dblink_fetch()

The previous patch was missing dblink--1.1--1.2.sql and
dblink--1.2.sql. I have added them, so it should apply cleanly against
git master, but not done any actual review yet.

This extension format is still incorrect, as there is no need for
dblink--1.1.sql as on the latest version of an extension needs to be
supported, and Makefile needs to be updated in consequence. Please
find attached an updated patch.

+-- fetch using anyelement, which will change the column names
 SELECT *
-FROM dblink_fetch('rmt_foo_cursor',4) AS t(a int, b text, c text[]);
- a | b |     c
----+---+------------
- 4 | e | {a4,b4,c4}
- 5 | f | {a5,b5,c5}
- 6 | g | {a6,b6,c6}
- 7 | h | {a7,b7,c7}
+FROM dblink_fetch('rmt_foo_cursor',4,null::foo) AS t;
+ f1 | f2 |     f3
+----+----+------------
+  4 | e  | {a4,b4,c4}
+  5 | f  | {a5,b5,c5}
+  6 | g  | {a6,b6,c6}
+  7 | h  | {a7,b7,c7}
 (4 rows)

Why isn't this test case not left alone? It looks like a regression to
me to not test it.
Regards,
--
Michael

On Mon, Jul 6, 2015 at 10:00 AM, Joe Conway <mail@joeconway.com> wrote:

I wonder if it isn't better to just loop through all the args with
get_fn_expr_argtype() every time and then test for the exact signature
match? Another alternative might be to create a wrapper C function for
each variant SQL function, but that seems like it could also get
messy, especially with dblink_record_internal() since we would have to
deal with every variant of all the external facing callers.
Thoughts?

Yeah, particularly the use of first_optarg makes things harder to
follow in the code with this patch. A C wrapper has the disadvantage
to decentralize the argument checks to many places making the flow
harder to follow hence using get_fn_expr_argtype() with PG_NARGS would
be the way to go, at least to me. This way, you can easily find how
many arguments there are, and which value is assigned to which
variable before moving on to the real processing.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 6, 2015 at 4:23 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Mon, Jul 6, 2015 at 10:00 AM, Joe Conway <mail@joeconway.com> wrote:

I wonder if it isn't better to just loop through all the args with
get_fn_expr_argtype() every time and then test for the exact signature
match? Another alternative might be to create a wrapper C function for
each variant SQL function, but that seems like it could also get
messy, especially with dblink_record_internal() since we would have to
deal with every variant of all the external facing callers.
Thoughts?

Yeah, particularly the use of first_optarg makes things harder to
follow in the code with this patch. A C wrapper has the disadvantage
to decentralize the argument checks to many places making the flow
harder to follow hence using get_fn_expr_argtype() with PG_NARGS would
be the way to go, at least to me. This way, you can easily find how
many arguments there are, and which value is assigned to which
variable before moving on to the real processing.

Just to be clear I mean that:
if (PG_NARGS() == 5)
{
if (get_fn_expr_argtype(fcinfo->flinfo, 1) == TYPEOID)
var = PG_GETARG_BOOL(1)
[...]
}
else if (PG_NARGS() == 4)
{
[...]
}
else
[...]
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Feb 19, 2015 at 4:06 PM, Corey Huinker <corey.huinker@gmail.com> wrote:

In the course of writing a small side project which hopefully will make its
way onto pgxn soon, I was writing functions that had a polymorphic result
set.

create function foo( p_row_type anyelement, p_param1 ...) returns setof
anyelement

Inside that function would be multiple calls to dblink() in both synchronous
and async modes. It is a requirement of the function that each query return
a result set conforming to the structure passed into p_row_type, but there
was no way for me to express that.

Unfortunately, there's no way to say

select * from dblink_get_result('connname') as <polymorphic record type>;

Instead, I had to generate the query as a string like this.

with x as (
select a.attname || ' ' || pg_catalog.format_type(a.atttypid,
a.atttypmod) as sql_text
from pg_catalog.pg_attribute a
where a.attrelid = pg_typeof(p_row_type)::text::regclass
and a.attisdropped is false
and a.attnum > 0
order by a.attnum )
select format('select * from dblink_get_result($1) as
t(%s)',string_agg(x.sql_text,','))
from x;

Moreover, I'm now executing this string dynamically, incurring reparsing and
replanning each time (and if all goes well, this would be executed many
times). Granted, I could avoid that by rewriting the stored procedure in C
and using prepared statements (not available in PL/PGSQL), but it seemed a
shame that dblink couldn't itself handle this polymorphism.

So with a little help, we were able to come up with polymorphic set
returning dblink functions.

Below is the results of the patch applied to a stock 9.4 installation.

[local]:ubuntu@dblink_test# create extension dblink;
CREATE EXTENSION
Time: 12.778 ms
[local]:ubuntu@dblink_test# \df dblink
List of functions
Schema | Name | Result data type | Argument data types |
Type
--------+--------+------------------+---------------------------------+--------
public | dblink | SETOF record | text |
normal
public | dblink | SETOF anyelement | text, anyelement |
normal
public | dblink | SETOF record | text, boolean |
normal
public | dblink | SETOF anyelement | text, boolean, anyelement |
normal
public | dblink | SETOF record | text, text |
normal
public | dblink | SETOF anyelement | text, text, anyelement |
normal
public | dblink | SETOF record | text, text, boolean |
normal
public | dblink | SETOF anyelement | text, text, boolean, anyelement |
normal
(8 rows)

sorry for the late reply. I'm a little concerned about the state of
overloading here. If I'm not mistaken, you may have introduced a
pretty serious backwards compatibility issue. Having the two
signatures (text, anyelement) and (text, boolean) will now fail
anytime (unknown, unknown) is passed, and that's a pretty common
invocation. If I'm right, quickly scanning the function list, I don't
think there's an easy solution to this issue other than adding an
alternately named call.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/2015 12:39 AM, Michael Paquier wrote:

Yeah, particularly the use of first_optarg makes things harder
to follow in the code with this patch. A C wrapper has the
disadvantage to decentralize the argument checks to many places
making the flow harder to follow hence using
get_fn_expr_argtype() with PG_NARGS would be the way to go, at
least to me. This way, you can easily find how many arguments
there are, and which value is assigned to which variable before
moving on to the real processing.

Just to be clear I mean that: if (PG_NARGS() == 5) { if
(get_fn_expr_argtype(fcinfo->flinfo, 1) == TYPEOID) var =
PG_GETARG_BOOL(1) [...]

Actually, I had in mind something like:
8<---------------------
int i;
int numargs;
int *argtypes;

numargs = PG_NARGS();
argtypes = palloc(numargs * sizeof(int));
for (i = 0; i < numargs; i++)
argtypes[i] = get_fn_expr_argtype(fcinfo->flinfo, i);

if ((numargs == 4 || numargs == 5) &&
argtypes[0] == TEXTOID &&
argtypes[1] == TEXTOID &&
argtypes[2] == INT4OID &&
argtypes[3] == BOOLOID)
{
[...]
}
else if ((numargs == 3 || numargs == 4) &&
argtypes[0] == TEXTOID &&
argtypes[1] == INT4OID &&
argtypes[2] == BOOLOID)
{
[...]
8<---------------------
etc.

If the highest number of arguments is always checked first, the
pattern is not ambiguous even with the extra anyelement.

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVmovZAAoJEDfy90M199hlzDsP/j/v76Ecv3VtZ+02Lo9mUVp6
mf+ZTLgw2OE4sNI6PL2MeVzu5ayZAkHxPRAah9rR1A7nDNEyZovi3dymFRHeiJ6Z
rowjkdZiRX/xlV5s6RHrdmX6DkVkBeGcKMuIKB/Tud2uPCiuBULkcUuD1OvlxsCs
W0E+hsuYmpGtsH8Vth+ciKiBUDX/BVWCOnqZXISRf6BZ5BjzITOEuCyn4EChx2o4
9gOGPTf3P/4I3buuDuV+DEmO1N4L07VvSWb9e92NrdS3VI1ae8YJu3u248WVmdPY
+qHg0J7jLGYBFRZ+isC7p8OX7PANCm88GvMCqklZdPo+/76n4J6x5MJfjinQEfXN
rbScwRh3O1DCimw404WqYSGKGEcX7MtUt98h+//nMft3aEz1gRnsuopnM/eRmQcS
wYlbBYon5YEdamx2o5AP6NX5zFU+6HBcKcznCFQtcsaJeh03yz9zILuCWxg4dWNj
T5aVCYu58PN4ELKP3yF5wN2UUSE4/OZJHgvIrHF8LiDOSdygvfADdUAmfD0sry48
tjbL9GC7JwHxqQ8qYktDMogxZo+s3TBsmw3NsnYXrNYwObbGCP9J0K0zV76ukMEc
V1qiMXD4/gddkKXJz0cVfcActrDqEltKDPTCdhLpoc4Gb59mrohgk+j75f2af14V
+n/AvaymgwdKjQpBhaTb
=tESF
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Joe Conway <mail@joeconway.com> writes:

Actually, I had in mind something like:
8<---------------------
int i;
int numargs;
int *argtypes;

numargs = PG_NARGS();
argtypes = palloc(numargs * sizeof(int));
for (i = 0; i < numargs; i++)
argtypes[i] = get_fn_expr_argtype(fcinfo->flinfo, i);

if ((numargs == 4 || numargs == 5) &&
argtypes[0] == TEXTOID &&
argtypes[1] == TEXTOID &&
argtypes[2] == INT4OID &&
argtypes[3] == BOOLOID)
{
[...]
}
else if ((numargs == 3 || numargs == 4) &&
argtypes[0] == TEXTOID &&
argtypes[1] == INT4OID &&
argtypes[2] == BOOLOID)
{
[...]
8<---------------------
etc.

If the set of allowed argument-type combinations is so easily enumerable,
I don't understand why this is being done at all. Create a separate SQL
function for each combination. You can still let the called C functions
call a common implementation routine if that's helpful.

However, this might all be moot in view of Merlin's objection. It is
definitely completely uncool to have both of these:

public | dblink | SETOF anyelement | text, anyelement | normal
public | dblink | SETOF record | text, boolean | normal

It's quite unclear which one will get called for cases like, say, second
argument is a domain over boolean. And even if the second arg is just a
boolean, maybe the user wanted the first case --- how will he get that
behavior, if so? These need to have different names, and that might well
help resolve the implementation-level issue...

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

On Mon, Jul 6, 2015 at 9:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Joe Conway <mail@joeconway.com> writes:

Actually, I had in mind something like:
8<---------------------
int i;
int numargs;
int *argtypes;

numargs = PG_NARGS();
argtypes = palloc(numargs * sizeof(int));
for (i = 0; i < numargs; i++)
argtypes[i] = get_fn_expr_argtype(fcinfo->flinfo, i);

if ((numargs == 4 || numargs == 5) &&
argtypes[0] == TEXTOID &&
argtypes[1] == TEXTOID &&
argtypes[2] == INT4OID &&
argtypes[3] == BOOLOID)
{
[...]
}
else if ((numargs == 3 || numargs == 4) &&
argtypes[0] == TEXTOID &&
argtypes[1] == INT4OID &&
argtypes[2] == BOOLOID)
{
[...]
8<---------------------
etc.

If the set of allowed argument-type combinations is so easily enumerable,
I don't understand why this is being done at all. Create a separate SQL
function for each combination. You can still let the called C functions
call a common implementation routine if that's helpful.

However, this might all be moot in view of Merlin's objection. It is
definitely completely uncool to have both of these:

public | dblink | SETOF anyelement | text, anyelement | normal
public | dblink | SETOF record | text, boolean | normal

It's quite unclear which one will get called for cases like, say, second
argument is a domain over boolean. And even if the second arg is just a
boolean, maybe the user wanted the first case --- how will he get that
behavior, if so? These need to have different names, and that might well
help resolve the implementation-level issue...

yup, and at least one case now fails where previously it ran through:
postgres=# select * from dblink('a', 'b', 'c');
ERROR: function dblink(unknown, unknown, unknown) is not unique

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 6, 2015 at 10:08 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/2015 12:39 AM, Michael Paquier wrote:

Yeah, particularly the use of first_optarg makes things harder
to follow in the code with this patch. A C wrapper has the
disadvantage to decentralize the argument checks to many places
making the flow harder to follow hence using
get_fn_expr_argtype() with PG_NARGS would be the way to go, at
least to me. This way, you can easily find how many arguments
there are, and which value is assigned to which variable before
moving on to the real processing.

Just to be clear I mean that: if (PG_NARGS() == 5) { if
(get_fn_expr_argtype(fcinfo->flinfo, 1) == TYPEOID) var =
PG_GETARG_BOOL(1) [...]

Actually, I had in mind something like:
8<---------------------
int i;
int numargs;
int *argtypes;

numargs = PG_NARGS();
argtypes = palloc(numargs * sizeof(int));
for (i = 0; i < numargs; i++)
argtypes[i] = get_fn_expr_argtype(fcinfo->flinfo, i);

if ((numargs == 4 || numargs == 5) &&
argtypes[0] == TEXTOID &&
argtypes[1] == TEXTOID &&
argtypes[2] == INT4OID &&
argtypes[3] == BOOLOID)
{
[...]
}
else if ((numargs == 3 || numargs == 4) &&
argtypes[0] == TEXTOID &&
argtypes[1] == INT4OID &&
argtypes[2] == BOOLOID)
{
[...]
8<---------------------
etc.

If the highest number of arguments is always checked first, the
pattern is not ambiguous even with the extra anyelement.

Utterly reasonable and do-able. I'll get to work on that soonish, pending
resolution of other's concerns.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/2015 07:37 AM, Merlin Moncure wrote:

yup, and at least one case now fails where previously it ran
through: postgres=# select * from dblink('a', 'b', 'c'); ERROR:
function dblink(unknown, unknown, unknown) is not unique

Hmm, that is an issue, possibly a fatal one.

When Cory first mentioned this to me over a year ago we discussed some
other, arguably better and more generic solutions. One was to build
support for:

SELECT * FROM srf() AS TYPE OF(foo);

The second idea I think is actually SQL standard if I recall correctly:

SELECT * FROM CAST(srf() AS foo) x;

Currently this works:

8<--------------------
select *
from cast (row(11,'l','{a11,b11,c11}') as foo);
f1 | f2 | f3
- ----+----+---------------
11 | l | {a11,b11,c11}
(1 row)
8<--------------------

But this fails:

8<--------------------
select *
from cast
(dblink('dbname=contrib_regression','select * from foo') as foo);
ERROR: cannot cast type record to foo
8<--------------------

Comments in the source have this to say:

8<--------------------
/*
* coerce_record_to_complex
* Coerce a RECORD to a specific composite type.
*
* Currently we only support this for inputs that are RowExprs or
* whole-row Vars.
*/
8<--------------------

That explains why the first example works while the second does not.
I'm not sure how hard it would be to fix that, but it appears that
that is where we should focus.

Joe

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVmpZDAAoJEDfy90M199hlIUsQAKcYeOXzzrlZjCeDSdIdx8YM
lAZ2kxjPwy5tLwm/WL1HwAM0epSBIMneF0JZuysFn77+bvbXId6LWtxkFKNakypP
tM5Ep0BjUFXgvzM3DVL0u9dkvrgcKlnwILsBjiWXqUVd/x8o2VAqfSOwfKG2SNQU
zo0l6u7ZeKY+gkFDsKU7eCo01dEI7MjGelKynRNE8TIpAJszNMsPpA4Y4WVRjbDT
GvA4O7f1Cws8Ewszle4gRWjx6TZ0mJVlt/FriFgtoRepoJjEalb5nfGLulx47Veq
iMFLbZr8xxc5u/ncR8bgi3Vc9g8H3eZsV6S85JeGuBS6cJOuw561gH5LkyEi5oeW
NHjZgf3oUnMZg1JE5cCGOyTQ/E/63itTor767f6KpaP/oEXckSCIUTh7azvD89sm
FD2udE3UWgC+16U11Ru+3TrRz11ETodF4TeW674CW3zA39dYjj3Us/PG6SDbM6zm
INO+pBvDIdhVaYvqy1yRqGMzgoQyCIAuI6sP+Q6CYqOd1Fdl42zPKGZ3F2SLEjq5
RfTKywrE1VHv1eafa6lCs5yaR7Jzr5FyRKFw9ob+TixN+x7vf6Gwcb9hkGzs9t2J
7NIiZ++U02fgVVVJGJ2VZ24gZVytP6sahq6Z6ccWacwic7lL7Us0mWgOKLzU1Umj
NLwHJoDkgV7SgGWVMsE/
=aRTA
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/2015 07:37 AM, Merlin Moncure wrote:

yup, and at least one case now fails where previously it ran
through: postgres=# select * from dblink('a', 'b', 'c'); ERROR:
function dblink(unknown, unknown, unknown) is not unique

Hmm, that is an issue, possibly a fatal one.

When Cory first mentioned this to me over a year ago we discussed some
other, arguably better and more generic solutions. One was to build
support for:

SELECT * FROM srf() AS TYPE OF(foo);

The second idea I think is actually SQL standard if I recall correctly:

SELECT * FROM CAST(srf() AS foo) x;

Currently this works:

8<--------------------
select *
from cast (row(11,'l','{a11,b11,c11}') as foo);
f1 | f2 | f3
- ----+----+---------------
11 | l | {a11,b11,c11}
(1 row)
8<--------------------

But this fails:

8<--------------------
select *
from cast
(dblink('dbname=contrib_regression','select * from foo') as foo);
ERROR: cannot cast type record to foo
8<--------------------

Comments in the source have this to say:

8<--------------------
/*
* coerce_record_to_complex
* Coerce a RECORD to a specific composite type.
*
* Currently we only support this for inputs that are RowExprs or
* whole-row Vars.
*/
8<--------------------

That explains why the first example works while the second does not.
I'm not sure how hard it would be to fix that, but it appears that
that is where we should focus.

Yeah. FWIW, here's my 0.02$: I use dblink all the time, for all
kinds of reasons, vastly preferring to have control over the query
string (vs. FDW type solutions). I have two basic gripes with it. #1
is that remote queries are not cancelled over all call sites when
cancelled locally (off-topic for this thread) and #2 is that the SRF
record describing mechanics are not abstractable because of using
syntax to describe the record. Corey's proposal, overloading issues
aside, appears to neatly deal with this problem because anyelement can
be passed down through a wrapping API.

IOW, I'd like to do:
CREATE FUNCTION remote_call(...) RETURNS ... AS
$$
SELECT dblink(...) AS r(...)
$$ language sql;

...which can't be done (discounting dynamic sql acrobatics) because of
the syntax based expression of the 'r' record. So I like Corey's
idea...I just think the functions need to be named differently (maybe
to 'dblink_any', and dblink_get_result_any'?). TBH, to do better
than that you'd need SQL level support for handling the return type in
the vein of NEW/OLD. For fun, let's call it 'OUT'...then you could:

SELECT * FROM remote_call(...) RETURNS SETOF foo;

Inside remote_call, you'd see something like:

SELECT dblink(...) AS OUT;

As to the proposed syntax, I would vote to support the SQL standard
variant if it could be handled during parse. I don't see what AS TYPE
OF really buys you because FWICT it does not support wrapping.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 6, 2015 at 11:33 AM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/2015 07:37 AM, Merlin Moncure wrote:

yup, and at least one case now fails where previously it ran
through: postgres=# select * from dblink('a', 'b', 'c'); ERROR:
function dblink(unknown, unknown, unknown) is not unique

Hmm, that is an issue, possibly a fatal one.

When Cory first mentioned this to me over a year ago we discussed some
other, arguably better and more generic solutions. One was to build
support for:

SELECT * FROM srf() AS TYPE OF(foo);

The second idea I think is actually SQL standard if I recall correctly:

SELECT * FROM CAST(srf() AS foo) x;

Currently this works:

8<--------------------
select *
from cast (row(11,'l','{a11,b11,c11}') as foo);
f1 | f2 | f3
- ----+----+---------------
11 | l | {a11,b11,c11}
(1 row)
8<--------------------

But this fails:

8<--------------------
select *
from cast
(dblink('dbname=contrib_regression','select * from foo') as foo);
ERROR: cannot cast type record to foo
8<--------------------

Comments in the source have this to say:

8<--------------------
/*
* coerce_record_to_complex
* Coerce a RECORD to a specific composite type.
*
* Currently we only support this for inputs that are RowExprs or
* whole-row Vars.
*/
8<--------------------

That explains why the first example works while the second does not.
I'm not sure how hard it would be to fix that, but it appears that
that is where we should focus.

Yeah. FWIW, here's my 0.02$: I use dblink all the time, for all
kinds of reasons, vastly preferring to have control over the query
string (vs. FDW type solutions). I have two basic gripes with it. #1
is that remote queries are not cancelled over all call sites when
cancelled locally (off-topic for this thread) and #2 is that the SRF
record describing mechanics are not abstractable because of using
syntax to describe the record. Corey's proposal, overloading issues
aside, appears to neatly deal with this problem because anyelement can
be passed down through a wrapping API.

IOW, I'd like to do:
CREATE FUNCTION remote_call(...) RETURNS ... AS
$$
SELECT dblink(...) AS r(...)
$$ language sql;

...which can't be done (discounting dynamic sql acrobatics) because of
the syntax based expression of the 'r' record. So I like Corey's
idea...I just think the functions need to be named differently (maybe
to 'dblink_any', and dblink_get_result_any'?). TBH, to do better
than that you'd need SQL level support for handling the return type in
the vein of NEW/OLD. For fun, let's call it 'OUT'...then you could:

SELECT * FROM remote_call(...) RETURNS SETOF foo;

Inside remote_call, you'd see something like:

SELECT dblink(...) AS OUT;

As to the proposed syntax, I would vote to support the SQL standard
variant if it could be handled during parse. I don't see what AS TYPE
OF really buys you because FWICT it does not support wrapping.

merlin

Your experiences with dblink are very similar to mine.

The whole issue arose for me as an outcropping of my Poor Man's Parallel
Processing extension (still not released but currently working for us in
production internally).

At some point I had to do dblink_get_result(...) as t(...) and not only did
I have to render the structure as a string, I was going to have to execute
that SQL dynamically (because plpgsql lacks a PREPARE statement) or I was
going to have to re-code in C or plv8. Overall those calls aren't terribly
expensive (it's working in production - for us - without this dblink
modification), but a cleaner solution would be better.

On Mon, Jul 6, 2015 at 11:13 AM, Corey Huinker <corey.huinker@gmail.com> wrote:

On Mon, Jul 6, 2015 at 11:33 AM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/2015 07:37 AM, Merlin Moncure wrote:

yup, and at least one case now fails where previously it ran
through: postgres=# select * from dblink('a', 'b', 'c'); ERROR:
function dblink(unknown, unknown, unknown) is not unique

Hmm, that is an issue, possibly a fatal one.

When Cory first mentioned this to me over a year ago we discussed some
other, arguably better and more generic solutions. One was to build
support for:

SELECT * FROM srf() AS TYPE OF(foo);

The second idea I think is actually SQL standard if I recall correctly:

SELECT * FROM CAST(srf() AS foo) x;

Currently this works:

8<--------------------
select *
from cast (row(11,'l','{a11,b11,c11}') as foo);
f1 | f2 | f3
- ----+----+---------------
11 | l | {a11,b11,c11}
(1 row)
8<--------------------

But this fails:

8<--------------------
select *
from cast
(dblink('dbname=contrib_regression','select * from foo') as foo);
ERROR: cannot cast type record to foo
8<--------------------

Comments in the source have this to say:

8<--------------------
/*
* coerce_record_to_complex
* Coerce a RECORD to a specific composite type.
*
* Currently we only support this for inputs that are RowExprs or
* whole-row Vars.
*/
8<--------------------

That explains why the first example works while the second does not.
I'm not sure how hard it would be to fix that, but it appears that
that is where we should focus.

Yeah. FWIW, here's my 0.02$: I use dblink all the time, for all
kinds of reasons, vastly preferring to have control over the query
string (vs. FDW type solutions). I have two basic gripes with it. #1
is that remote queries are not cancelled over all call sites when
cancelled locally (off-topic for this thread) and #2 is that the SRF
record describing mechanics are not abstractable because of using
syntax to describe the record. Corey's proposal, overloading issues
aside, appears to neatly deal with this problem because anyelement can
be passed down through a wrapping API.

IOW, I'd like to do:
CREATE FUNCTION remote_call(...) RETURNS ... AS
$$
SELECT dblink(...) AS r(...)
$$ language sql;

...which can't be done (discounting dynamic sql acrobatics) because of
the syntax based expression of the 'r' record. So I like Corey's
idea...I just think the functions need to be named differently (maybe
to 'dblink_any', and dblink_get_result_any'?). TBH, to do better
than that you'd need SQL level support for handling the return type in
the vein of NEW/OLD. For fun, let's call it 'OUT'...then you could:

SELECT * FROM remote_call(...) RETURNS SETOF foo;

Inside remote_call, you'd see something like:

SELECT dblink(...) AS OUT;

As to the proposed syntax, I would vote to support the SQL standard
variant if it could be handled during parse. I don't see what AS TYPE
OF really buys you because FWICT it does not support wrapping.

merlin

Your experiences with dblink are very similar to mine.

The whole issue arose for me as an outcropping of my Poor Man's Parallel
Processing extension (still not released but currently working for us in
production internally).

At some point I had to do dblink_get_result(...) as t(...) and not only did
I have to render the structure as a string, I was going to have to execute
that SQL dynamically (because plpgsql lacks a PREPARE statement) or I was
going to have to re-code in C or plv8. Overall those calls aren't terribly
expensive (it's working in production - for us - without this dblink
modification), but a cleaner solution would be better.

Another option is to handle the intermediate result in json if you're
willing to sacrifice a little bit of performance. For example,
suppose you wanted to log every dblink call through an wrapper without
giving up the ability to handle arbitrary result sets:

CREATE OR REPLACE FUNCTION dblink_log(_query TEXT) RETURNS SETOF JSON AS
$$
BEGIN
RAISE WARNING 'got dblink %', _query;

RETURN QUERY SELECT * FROM dblink(
'host=localhost',
format('SELECT row_to_json(q) from (%s) q', _query))
AS R(j json);
END;
$$ LANGUAGE PLPGSQL;

postgres=# select * from dblink_log('select 0 as value');

WARNING: got dblink select 0 as value
dblink_log
─────────────
{"value":0}
(1 row)

With json, you have a number of good options to convert back to a
record. For example,

postgres=# CREATE TYPE foo AS (value int);
CREATE TYPE

SELECT p.*
FROM dblink_log('SELECT s AS value FROM generate_series(1,3) s') j
CROSS JOIN LATERAL json_populate_record(null::foo, j) p;

WARNING: got dblink select s as value from generate_series(1,3) s
value
───────
1
2
3
(3 rows)

Note, I still support the case behind your patch, but, if it uh,
doesn't make it through, it's good to have options :-).

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 6, 2015 at 3:52 PM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Mon, Jul 6, 2015 at 11:13 AM, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Mon, Jul 6, 2015 at 11:33 AM, Merlin Moncure <mmoncure@gmail.com>

wrote:

On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/2015 07:37 AM, Merlin Moncure wrote:

yup, and at least one case now fails where previously it ran
through: postgres=# select * from dblink('a', 'b', 'c'); ERROR:
function dblink(unknown, unknown, unknown) is not unique

Hmm, that is an issue, possibly a fatal one.

When Cory first mentioned this to me over a year ago we discussed some
other, arguably better and more generic solutions. One was to build
support for:

SELECT * FROM srf() AS TYPE OF(foo);

The second idea I think is actually SQL standard if I recall

correctly:

SELECT * FROM CAST(srf() AS foo) x;

Currently this works:

8<--------------------
select *
from cast (row(11,'l','{a11,b11,c11}') as foo);
f1 | f2 | f3
- ----+----+---------------
11 | l | {a11,b11,c11}
(1 row)
8<--------------------

But this fails:

8<--------------------
select *
from cast
(dblink('dbname=contrib_regression','select * from foo') as foo);
ERROR: cannot cast type record to foo
8<--------------------

Comments in the source have this to say:

8<--------------------
/*
* coerce_record_to_complex
* Coerce a RECORD to a specific composite type.
*
* Currently we only support this for inputs that are RowExprs or
* whole-row Vars.
*/
8<--------------------

That explains why the first example works while the second does not.
I'm not sure how hard it would be to fix that, but it appears that
that is where we should focus.

Yeah. FWIW, here's my 0.02$: I use dblink all the time, for all
kinds of reasons, vastly preferring to have control over the query
string (vs. FDW type solutions). I have two basic gripes with it. #1
is that remote queries are not cancelled over all call sites when
cancelled locally (off-topic for this thread) and #2 is that the SRF
record describing mechanics are not abstractable because of using
syntax to describe the record. Corey's proposal, overloading issues
aside, appears to neatly deal with this problem because anyelement can
be passed down through a wrapping API.

IOW, I'd like to do:
CREATE FUNCTION remote_call(...) RETURNS ... AS
$$
SELECT dblink(...) AS r(...)
$$ language sql;

...which can't be done (discounting dynamic sql acrobatics) because of
the syntax based expression of the 'r' record. So I like Corey's
idea...I just think the functions need to be named differently (maybe
to 'dblink_any', and dblink_get_result_any'?). TBH, to do better
than that you'd need SQL level support for handling the return type in
the vein of NEW/OLD. For fun, let's call it 'OUT'...then you could:

SELECT * FROM remote_call(...) RETURNS SETOF foo;

Inside remote_call, you'd see something like:

SELECT dblink(...) AS OUT;

As to the proposed syntax, I would vote to support the SQL standard
variant if it could be handled during parse. I don't see what AS TYPE
OF really buys you because FWICT it does not support wrapping.

merlin

Your experiences with dblink are very similar to mine.

The whole issue arose for me as an outcropping of my Poor Man's Parallel
Processing extension (still not released but currently working for us in
production internally).

At some point I had to do dblink_get_result(...) as t(...) and not only

did

I have to render the structure as a string, I was going to have to

execute

that SQL dynamically (because plpgsql lacks a PREPARE statement) or I was
going to have to re-code in C or plv8. Overall those calls aren't

terribly

expensive (it's working in production - for us - without this dblink
modification), but a cleaner solution would be better.

Another option is to handle the intermediate result in json if you're
willing to sacrifice a little bit of performance. For example,
suppose you wanted to log every dblink call through an wrapper without
giving up the ability to handle arbitrary result sets:

CREATE OR REPLACE FUNCTION dblink_log(_query TEXT) RETURNS SETOF JSON AS
$$
BEGIN
RAISE WARNING 'got dblink %', _query;

RETURN QUERY SELECT * FROM dblink(
'host=localhost',
format('SELECT row_to_json(q) from (%s) q', _query))
AS R(j json);
END;
$$ LANGUAGE PLPGSQL;

postgres=# select * from dblink_log('select 0 as value');

WARNING: got dblink select 0 as value
dblink_log
─────────────
{"value":0}
(1 row)

With json, you have a number of good options to convert back to a
record. For example,

postgres=# CREATE TYPE foo AS (value int);
CREATE TYPE

SELECT p.*
FROM dblink_log('SELECT s AS value FROM generate_series(1,3) s') j
CROSS JOIN LATERAL json_populate_record(null::foo, j) p;

WARNING: got dblink select s as value from generate_series(1,3) s
value
───────
1
2
3
(3 rows)

Note, I still support the case behind your patch, but, if it uh,
doesn't make it through, it's good to have options :-).

merlin

(again, more backstory to enhance other's understanding of the issue)

In the earlier iterations of PMPP, I had it simply wrap all queries sent to
the remote server inside a 'SELECT * from row_to_json(...)'.

The serialization/deserialization was a performance hit, offset slightly by
having the RETURN QUERY SELECT json_field from dblink_get_result('c1') as
t(json_field json) be a static (reusable) query.

The ugliness of decomposing the fields from json was no fun, and Merlin's
suggestion of json_populate_record ( I don't remember if that function
existed at the time...) would get around that, albeit with another
performance hit.

On Mon, Jul 6, 2015 at 11:52 PM, Joe Conway <mail@joeconway.com> wrote:

That explains why the first example works while the second does not.
I'm not sure how hard it would be to fix that, but it appears that
that is where we should focus.

Wouldn't it be fine if we drop some of the functions proposed without
impacting the feature? Most of the functions overlap with each other,
making us see the limitations we see.

Hence, wouldn't it be enough to just have this set of functions in the patch?
dblink_get_result(text, bool, anyelement)
dblink (text, text, boolean, anyelement)
dblink_fetch (text, text, int, boolean, anyelement)
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/07/2015 10:22 PM, Michael Paquier wrote:

On Mon, Jul 6, 2015 at 11:52 PM, Joe Conway <mail@joeconway.com>
wrote:

That explains why the first example works while the second does
not. I'm not sure how hard it would be to fix that, but it
appears that that is where we should focus.

Wouldn't it be fine if we drop some of the functions proposed
without impacting the feature? Most of the functions overlap with
each other, making us see the limitations we see.

Hence, wouldn't it be enough to just have this set of functions in
the patch? dblink_get_result(text, bool, anyelement) dblink (text,
text, boolean, anyelement) dblink_fetch (text, text, int, boolean,
anyelement)

I think new using function names is better especially if we are only
going to support a subset. I have no idea what to call them however.
Did someone else suggest dblink_any(), etc?

I also think that the ultimately best solution is (what I believe to
be spec compliant) SRF casting, but I guess that could be a task for a
later day.

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVnT30AAoJEDfy90M199hl/dcP/29Byp5SE0TPE5EHeJwS6MDf
76KpsiyMZkxddBaiXiPbgrWEBZHdGExyzYj2qO9utPuFlzVTZpAbVcoFXaUg9Ak/
VMKkoxSB1iq1E1Pu64K26xvCU0GzMW1bazcGo4263iSlDjiRCB+CkL5UokEqSmvb
c9u7UZ3sdgn34C9iF3Z6pHa5GQoYB+i3kCxFuCdYMyHFZYdA0EtDvFNCoCNsWUCW
783IiQbeKCcj+j0bJ5v8lGfvHGfAJV0C0xiYiLios2nyClOvIQQQv2YN84BWDM6F
oWWJoExd51iL94RF/hRBJ/WXFFBfg/r30ULW5/dwhdp8cl1wPW+cY4srEHlTluZZ
KzoCujD5rhofJsS7tV9xs6tV4K/4/enknogHT0iWT89B/svAR52SUkRiH22mvhps
cpVVOsIP+ojmvmWW2p8jvq9ml5Ls6Z24AyRZ+OVZnVZGzAC9Z+NhwcjJaDWiWU2+
lgVuvIorYWZLHGzd6syrZKxKxdJRZ5ibYe2SN3Q2wlicZRPS5cXOgTfKoSduMTvY
GoNRPHQSPeuBNoq4pFUWN8EqH3oijN+PqdnIwc0HMaDyxR0AnISZiLwYWnY8WlqB
pdFV6tGsvsM6WbXTIqJ/3diRjRMl2/rtz6o8b3a1K7eL+PB7v5T0I+h9pwJQM2+a
WPffDIaY2Odnt9Axebu5
=CZ0t
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2015-07-08 17:12 GMT+02:00 Joe Conway <mail@joeconway.com>:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/07/2015 10:22 PM, Michael Paquier wrote:

On Mon, Jul 6, 2015 at 11:52 PM, Joe Conway <mail@joeconway.com>
wrote:

That explains why the first example works while the second does
not. I'm not sure how hard it would be to fix that, but it
appears that that is where we should focus.

Wouldn't it be fine if we drop some of the functions proposed
without impacting the feature? Most of the functions overlap with
each other, making us see the limitations we see.

Hence, wouldn't it be enough to just have this set of functions in
the patch? dblink_get_result(text, bool, anyelement) dblink (text,
text, boolean, anyelement) dblink_fetch (text, text, int, boolean,
anyelement)

I think new using function names is better especially if we are only
going to support a subset. I have no idea what to call them however.
Did someone else suggest dblink_any(), etc?

+1

Pavel

On Wed, Jul 8, 2015 at 10:12 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/07/2015 10:22 PM, Michael Paquier wrote:

On Mon, Jul 6, 2015 at 11:52 PM, Joe Conway <mail@joeconway.com>
wrote:

That explains why the first example works while the second does
not. I'm not sure how hard it would be to fix that, but it
appears that that is where we should focus.

Wouldn't it be fine if we drop some of the functions proposed
without impacting the feature? Most of the functions overlap with
each other, making us see the limitations we see.

Hence, wouldn't it be enough to just have this set of functions in
the patch? dblink_get_result(text, bool, anyelement) dblink (text,
text, boolean, anyelement) dblink_fetch (text, text, int, boolean,
anyelement)

I think new using function names is better especially if we are only
going to support a subset. I have no idea what to call them however.
Did someone else suggest dblink_any(), etc?

I also think that the ultimately best solution is (what I believe to
be spec compliant) SRF casting, but I guess that could be a task for a
later day.

totally agree. Even if we had SRF casting, OP's patch has value
because of abstraction benefits. Also given that we are in an
extension we can relax a bit about adding extra functions IMO.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 8, 2015 at 11:29 AM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Wed, Jul 8, 2015 at 10:12 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/07/2015 10:22 PM, Michael Paquier wrote:

On Mon, Jul 6, 2015 at 11:52 PM, Joe Conway <mail@joeconway.com>
wrote:

That explains why the first example works while the second does
not. I'm not sure how hard it would be to fix that, but it
appears that that is where we should focus.

Wouldn't it be fine if we drop some of the functions proposed
without impacting the feature? Most of the functions overlap with
each other, making us see the limitations we see.

Hence, wouldn't it be enough to just have this set of functions in
the patch? dblink_get_result(text, bool, anyelement) dblink (text,
text, boolean, anyelement) dblink_fetch (text, text, int, boolean,
anyelement)

I think new using function names is better especially if we are only
going to support a subset. I have no idea what to call them however.
Did someone else suggest dblink_any(), etc?

I also think that the ultimately best solution is (what I believe to
be spec compliant) SRF casting, but I guess that could be a task for a
later day.

totally agree. Even if we had SRF casting, OP's patch has value
because of abstraction benefits. Also given that we are in an
extension we can relax a bit about adding extra functions IMO.

merlin

I'm fine with separate functions, that's what I originally proposed to Joe
way-back when I was trying to figure out if such a thing was possible.

That would also allow me to move the rowtype parameter to the first
parameter, much more in line with other polymorphic functions. And that
*might* resolve the parameter ambiguity issue

Questions:
Would moving rowtype to the first parameter resolve the parameter
ambiguity issue?
Would we want new function names anyway?
How much of a goal is reducing function count?

The following suffixes all make a degree of sense to me:
_any()
_to_row()
_to_rowtype()
_to_recordset() -- borrowing from json[b]_to_recordsset() and
json[b]_populate_recordset(), the first functions polymorphic functions to
come to mind.

IMO, _to_recordset() would win on POLA, and _any() would win on terse.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/08/2015 08:51 AM, Corey Huinker wrote:

Questions: Would moving rowtype to the first parameter resolve the
parameter ambiguity issue?

Not for the existing functions but with new functions I don't think it
matters. You would know to always ignore either the first or last
argument when determining which variant was wanted.

Would we want new function names anyway?

Yes, see above

How much of a goal is reducing function count?

Do you mean reducing number of C functions or SQL functions?

The following suffixes all make a degree of sense to me: _any()
_to_row() _to_rowtype() _to_recordset() -- borrowing from
json[b]_to_recordsset() and json[b]_populate_recordset(), the first
functions polymorphic functions to come to mind.

IMO, _to_recordset() would win on POLA, and _any() would win on
terse.

The problem is that jsonb_to_recordset() does not behave like these
new dblink functions in that it requires a runtime column definition
list. It might be better to use something completely different, so I
think _any() or maybe _to_any() is better.

Any other ideas for names out there?

Joe

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVnU4kAAoJEDfy90M199hlxtIP/i9+ksY4Mq9lN0Ne+moLs3My
KY1lyQqXkynJpnbYArBPnmC8ejso/f9FpAfkoI8jA+YfzVLgN38aM/H5d6Kvpt2R
mA/Dpw7OpUrbZCsUT6JO7p0WRTqX2lNqX9FausgVXCTDXkYvKm3Vc3AgOUPVOfgv
BHuls6xlYtVbxJsQ3zm//sONwE6SmBexi6LWlzJiH3+UjfjYOEs2yj5aCObac+2+
2umrc3vfAPoCcXSEcMOwHYWBkwu1pxwtHrGObZYUt6pHCmNsj4o67AQv6z64x6fl
bRgvy/GOz2ict1DOs7kWha7Fvr9xC3FTyXxWaIpePo5mT92AzILp1L5+YgGZTxaA
jQISashYH5EAob7ow3hRJL2Gey7iQzgpHBClDlb/Tv4kDWV6BaBWaeQL2AUHEEGN
Y81hrQ6nsKnAQpPGUqvN0VHDUHn81S3T1SJZRptennGVqvuHrKwyVQj0yJo3wfcT
itnFZS2XmhNY11uVUZnZ0lZMClLn2jjedmNrfSHQPm+5EZBoW2B2QoXe/J/Oci1S
UEfl4IJyNgjAxYiG+7koAlo5DrxTPupVLWnoMxao+U71OOvU2Tzz6Jx/qV9+Rs2j
2web3tAKGyytRK/C+OO/dgjQsOdvR9D6lLp6l3GuC4q06KWe35bZuNJzACqQQaHj
7s3oZuTRKWqu4fHXW1om
=RxAo
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 8, 2015 at 12:21 PM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/08/2015 08:51 AM, Corey Huinker wrote:

Questions: Would moving rowtype to the first parameter resolve the
parameter ambiguity issue?

Not for the existing functions but with new functions I don't think it
matters. You would know to always ignore either the first or last
argument when determining which variant was wanted.

Would we want new function names anyway?

Yes, see above

How much of a goal is reducing function count?

Do you mean reducing number of C functions or SQL functions?

C functions. Was there a reason (performance, style, etc) to have only one
function per function name, and let it suss out which signature call
happened this time, as opposed to having the signatures with defaulted
values implemented as either as C wrappers or SQL wrappers to C function
which can then assume the full-blown signature?

I'm asking because if making the C code more straightforward is a goal, and
wrapper overhead is minimal, then that could be a separate patch, either
preceding or following the polymorphic one.

UPDATE:
After less than an hour of recoding for the 3 _any() functions with full
signatures, I noticed that the C code would be exactly the for the
non-anyelement sets if we implemented the signatures with omitted
parameters as SQL wrappers (of course, the _any() ones would call the C
function without STRICT mode).

So this patch would result in less C code while still adding 3 new
functions. Can anyone think of why that wouldn't be the best way to go?

The problem is that jsonb_to_recordset() does not behave like these
new dblink functions in that it requires a runtime column definition
list. It might be better to use something completely different, so I
think _any() or maybe _to_any() is better.

Any other ideas for names out there?

Joe

_any() is what I'm going with, sticking with trailing anyelement to
highlight the "it's just like the function without the _any" aspect.

I'm also remembering to drop the --1.1 and restore the regression test case
I hijacked.

On 07/18/2015 01:32 AM, Corey Huinker wrote:

So this patch would result in less C code while still adding 3 new
functions. Can anyone think of why that wouldn't be the best way to go?

Let's pursue the "CAST(srf() AS row_rtype)" syntax that Joe suggested
upthread
(/messages/by-id/559A9643.9070409@joeconway.com).
For some reason, the discussion went on around the details of the
submitted patch after that, even though everyone seemed to prefer the
CAST() syntax.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 29, 2015 at 3:48 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 07/18/2015 01:32 AM, Corey Huinker wrote:

So this patch would result in less C code while still adding 3 new
functions. Can anyone think of why that wouldn't be the best way to go?

Let's pursue the "CAST(srf() AS row_rtype)" syntax that Joe suggested
upthread (
/messages/by-id/559A9643.9070409@joeconway.com). For
some reason, the discussion went on around the details of the submitted
patch after that, even though everyone seemed to prefer the CAST() syntax.

- Heikki

I'm all for adding that syntax, but it wouldn't be useful for my purposes
unless row_rtype could be a variable, and my understanding is that it can't.

Corey Huinker <corey.huinker@gmail.com> writes:

On Wed, Jul 29, 2015 at 3:48 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

Let's pursue the "CAST(srf() AS row_rtype)" syntax that Joe suggested
upthread (
/messages/by-id/559A9643.9070409@joeconway.com). For
some reason, the discussion went on around the details of the submitted
patch after that, even though everyone seemed to prefer the CAST() syntax.

I'm all for adding that syntax, but it wouldn't be useful for my purposes
unless row_rtype could be a variable, and my understanding is that it can't.

Not sure why inserting a variable name is so much better than inserting a
type name?

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

On Wed, Jul 29, 2015 at 10:48 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Corey Huinker <corey.huinker@gmail.com> writes:

On Wed, Jul 29, 2015 at 3:48 AM, Heikki Linnakangas <hlinnaka@iki.fi>

wrote:

Let's pursue the "CAST(srf() AS row_rtype)" syntax that Joe suggested
upthread (
/messages/by-id/559A9643.9070409@joeconway.com).

For

some reason, the discussion went on around the details of the submitted
patch after that, even though everyone seemed to prefer the CAST()

syntax.

I'm all for adding that syntax, but it wouldn't be useful for my purposes
unless row_rtype could be a variable, and my understanding is that it

can't.

Not sure why inserting a variable name is so much better than inserting a
type name?

regards, tom lane

Apologies in advance if I'm going over things you already know. Just trying
to package up the problem statement into something easily digestible.

In a polymorphic function, I don't know the return type. It's whatever type
was specified on the function call.

Say I've written a function with a function like
outer_polymorphic_function(p_rowtype anyelement,p1 ,p2,p3, ...) returns
setof anyelement

And inside that function is a series (probably a handful, but potentially
thousands) of async dblink calls, and their corresponding calls to
dblink_get_result(), which currently return setof record, each of which
needs to be casted to whatever anyelement happens to be given this
execution.

Currently, I have to look up p_rowtype in pg_attribute and pg_class, render
the column specs as valid SQL, and compose the query as a string

fetch_values_query := 'select * from dblink_get_result($1) as t ( ' ||
'c1 type, c2 othertype, ... ' || ')';

and then execute that dynamically like so:

return query execute fetch_values_query using l_connection_name;

It would be nice if I didn't have to resort to dynamic SQL do to this.

If the CAST() function is implemented, but does not allow to cast as a
variable, then I'm in the same boat:

fetch_values_query := 'select * from CAST(dblink_get_result($1) as ' ||
pg_typeof(p_rowtype) || ')';

Admittedly, that's a bit cleaner, but I'm still executing that dynamic SQL
once per connection I made, and there could be a lot of them.

If there were dblink() functions that returned polymorphic results, it
would be a non issue:

dblink_send_query('conn1','select * from
thing_i_know_is_shaped_like_my_rowtype')
...
return query select * from dblink_get_result_any(p_rowtype,'conn1');

I'm all for the expanded capabilities of CAST(), but I have a specific need
for polymorphic dblink() functions.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/29/2015 08:56 AM, Corey Huinker wrote:

On Wed, Jul 29, 2015 at 10:48 AM, Tom Lane <tgl@sss.pgh.pa.us Not
sure why inserting a variable name is so much better than inserting
a type name?

In a polymorphic function, I don't know the return type. It's
whatever type was specified on the function call.

Say I've written a function with a function like
outer_polymorphic_function(p_rowtype anyelement,p1 ,p2,p3, ...)
returns setof anyelement

Remind me where you get p_rowtype at runtime again? At some point you
are still having to calculate it, no?

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVuPv8AAoJEDfy90M199hlYCwP/i0/berqnjXFch6zFM5TDZ6h
+UxQxMmVg933U6Cdoc+huMz8Hiotix76BZVgHOa8xI7Vktx1y5D7o7auczg31v4o
BkzbJFX9ziK5TXZm5wEvtTPNJOOq25AqvHzmrwr4JnPvyAOCKQASTqhIi95mdflZ
tGI+fuI4Dlkj76JaIuZjIh1rKMdycHsRmVfULVx6luR5LwzhX/iyW66pMkNHPYui
7K3DP2hF4HR6xoq4Jvj+HX1MSLLRAjm6+emx6YKnkSkxCQvL5EzwupWWhr7khrYk
QV0fwbAG5JtQJlid/DxezUFgpi2qs2AoPy2ub7JyEZjY0ULt4ehOx9/pzk0ATMNo
e3jB1H9BHTCiy5tY3VZBCe0uQ3lqiNalyUPcwYviS3yciuNg78rIkCQKe2KritZw
t0BY0SMASjbgIINlOLkDCg3HaYi3JujdbwSR5dG41RxaMej3MMieh3Opyg9bfZhI
TxWLsec7FPW/T23wPGk1aQZ7IbLRlOz95fJlAua6g1d5m6GWE3lyRQAQP+QFNWfX
/dmAy0Hvyp3a1wRsFrsG1W+GoyNV1pwfjXp+QlDDhGf8G/Q4l5s7OzZRcLs0O67Z
3K7Jn2k8ps4ZxA5yqRZdl3aAuaOpf3iqffQEeWQqXx7UAM5Sd8qorOJXpNhw+JtX
9W1YQ3eNgGkDfcOa9JLm
=P2ff
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Corey Huinker <corey.huinker@gmail.com> writes:

On Wed, Jul 29, 2015 at 10:48 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Not sure why inserting a variable name is so much better than inserting a
type name?

In a polymorphic function, I don't know the return type. It's whatever type
was specified on the function call.

Say I've written a function with a function like
outer_polymorphic_function(p_rowtype anyelement,p1 ,p2,p3, ...) returns
setof anyelement

And inside that function is a series (probably a handful, but potentially
thousands) of async dblink calls, and their corresponding calls to
dblink_get_result(), which currently return setof record, each of which
needs to be casted to whatever anyelement happens to be given this
execution.

Yeah. I would argue that the appropriate fix for that involves allowing
the "p_rowtype%TYPE" syntax to be used in the CAST. I think right now
you can only use %TYPE in DECLARE sections, but that's a limitation that
causes problems in more situations than just this one.

Mind you, making that work might be less than trivial :-( ... but it would
have uses well beyond dblink().

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

On Wed, Jul 29, 2015 at 12:14 PM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/29/2015 08:56 AM, Corey Huinker wrote:

On Wed, Jul 29, 2015 at 10:48 AM, Tom Lane <tgl@sss.pgh.pa.us Not
sure why inserting a variable name is so much better than inserting
a type name?

In a polymorphic function, I don't know the return type. It's
whatever type was specified on the function call.

Say I've written a function with a function like
outer_polymorphic_function(p_rowtype anyelement,p1 ,p2,p3, ...)
returns setof anyelement

Remind me where you get p_rowtype at runtime again? At some point you
are still having to calculate it, no?

Say I've got a table my_partitioned_table (key1 integer, key2 integer,
metric1 integer, metric2 integer);

And I've got many partitions on that table.

My code lets you do something like this:

select key1, key2, sum(metric1) as a_sum_of_sums, sum(metric2) as
another_sum_of_sums
from execute_buncha_queries(null::my_partitioned_table,
'connection_string_thats_just_a_loopback',
array['select key1, key2, sum(metric1),
sum(metric2) from my_partition_p1 group by 1,2',
'select key1, key2, sum(metric1),
sum(metric2) from my_partition_p2 group by 1,2',
...])
group by 1,2

All those queries happen to return a shape the same as
my_partitioned_table. The query takes the partially summed values, spread
out across a lot of processors, and does the lighter work of summing the
sums.

The function execute_buncha_queries fires off those string queries async,
enough to fill up X number of processors, fetches results as they happen,
and keeps feeding the processors queries until it runs out. But
execute_buncha_queries needs to send back results in the shape of whatever
value was passed in the first parameter.

I can't put a cast around execute_buncha_queries because the function won't
know how to cast the results coming back from dblink.

select * from
execute_lotta_queries(null::my_table_or_type,'connection_string_to_remote_db',
array['query 1','query 2','query 3'])

Now, it's up to the user to make sure that all the query strings return a
query of shape "my_table_or_type", but that's a runtime problem.
And obviously, there are a lot of connection strings, but that's too much
detail for the problem at hand.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/29/2015 09:40 AM, Corey Huinker wrote:

Say I've got a table my_partitioned_table (key1 integer, key2
integer, metric1 integer, metric2 integer);

And I've got many partitions on that table. My code lets you do
something like this:

select key1, key2, sum(metric1) as a_sum_of_sums, sum(metric2) as
another_sum_of_sums from
execute_buncha_queries(null::my_partitioned_table,
'connection_string_thats_just_a_loopback', array['select key1,
key2, sum(metric1), sum(metric2) from my_partition_p1 group by
1,2', 'select key1, key2, sum(metric1), sum(metric2) from
my_partition_p2 group by 1,2', ...]) group by 1,2

I can't put a cast around execute_buncha_queries because the
function won't know how to cast the results coming back from
dblink.

Ok, gotcha. So Tom's nearby comment about allowing the
"p_rowtype%TYPE" syntax to be used in the CAST is spot on (as usual).
In other words, to get a complete solution for you we would need to
make both things work, so you could do this inside plpgsql:

select * from cast(dblink(connstr, sql) as p_rowtype%TYPE);

where potentially connstr, sql, p_rowtype are all passed to plpgsql as
arguments. Correct?

Joe

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVuRMWAAoJEDfy90M199hlT5sP/3GuKbvZC7Ods3i2SqtTGbTo
raFiKM87ZznswldNjDHVBEp+OntXzb0JbPUf6n/YJoEJGWE95wb40jez5snAV9lO
aJ7CD9P235OgVh7QVTeWIW5Who8Yj1Xx6NF/Gz/06pGoAXQj1QoznnUPYixur4dT
znjWW3XY6eFpfLzIBKIJmcskOKezgqj2F/kRJpgGYVaEm3okVkjubDjmPM5Vbaaa
sd/lDI5ByceIImzL8LaFeBWwUGLYRsP02zamfPiz3p1zMb+ViBvS+NiBG0kMZMCt
bzy6g0kxbLaxkKy/oEQXqinCNY3hEn8eZ7w4Os/3Zk9PCacZAKDrXfqBDTuE6zio
wy/nwBnoTvdBSk0gl+MKoVlmoy0iAV7Hl/R/KtdNdpCTL4Ja6R5V2c/sjWazxAg4
PymaTXi4/SNWTFwAYGJUfGL+i3CMNQfp4U/tGTVPGFZ8thss7FTVNIVR6ZcAzuPM
EHYDZ8cGaewqDF/HdPlJl4ypxF3aS8tzzApiFVpu35+2WHEacwljDV40l8z9Ee1V
E7R0oxG55fgRJKvLSn5Oj59U2iBXgcu0zLLhBhaVyOYhcIZbWe6xvF1UN/RNcOuz
Se10lYSXCTmz3q61HyCNnWFcOtgYSeFA3Lc79vgxJoZWmwnpHYoFEjQxr3qHFeeK
svkoix7k7t7YZUXGebbg
=vM1F
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ok, gotcha. So Tom's nearby comment about allowing the
"p_rowtype%TYPE" syntax to be used in the CAST is spot on (as usual).
In other words, to get a complete solution for you we would need to
make both things work, so you could do this inside plpgsql:

select * from cast(dblink(connstr, sql) as p_rowtype%TYPE);

where potentially connstr, sql, p_rowtype are all passed to plpgsql as
arguments. Correct?

Correct.

On Wed, Jul 29, 2015 at 12:53 PM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/29/2015 09:40 AM, Corey Huinker wrote:

Say I've got a table my_partitioned_table (key1 integer, key2
integer, metric1 integer, metric2 integer);

And I've got many partitions on that table. My code lets you do
something like this:

select key1, key2, sum(metric1) as a_sum_of_sums, sum(metric2) as
another_sum_of_sums from
execute_buncha_queries(null::my_partitioned_table,
'connection_string_thats_just_a_loopback', array['select key1,
key2, sum(metric1), sum(metric2) from my_partition_p1 group by
1,2', 'select key1, key2, sum(metric1), sum(metric2) from
my_partition_p2 group by 1,2', ...]) group by 1,2

I can't put a cast around execute_buncha_queries because the
function won't know how to cast the results coming back from
dblink.

Ok, gotcha. So Tom's nearby comment about allowing the
"p_rowtype%TYPE" syntax to be used in the CAST is spot on (as usual).
In other words, to get a complete solution for you we would need to
make both things work, so you could do this inside plpgsql:

select * from cast(dblink(connstr, sql) as p_rowtype%TYPE);

Would this be a pl/pgsql only solution?

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Merlin Moncure <mmoncure@gmail.com> writes:

On Wed, Jul 29, 2015 at 12:53 PM, Joe Conway <mail@joeconway.com> wrote:

Ok, gotcha. So Tom's nearby comment about allowing the
"p_rowtype%TYPE" syntax to be used in the CAST is spot on (as usual).
In other words, to get a complete solution for you we would need to
make both things work, so you could do this inside plpgsql:

select * from cast(dblink(connstr, sql) as p_rowtype%TYPE);

Would this be a pl/pgsql only solution?

Well, it would depend on how we fixed %TYPE, but my thought is that
we should teach the core parser to accept variable%TYPE anywhere that
a suitable "variable" is in scope. The core already allows related
syntaxes in some utility commands, but not within DML commands.

(I am not sure offhand if the existing core syntax is exactly compatible
with what plpgsql does, though; and that could be a problem.)

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

I wrote:

Well, it would depend on how we fixed %TYPE, but my thought is that
we should teach the core parser to accept variable%TYPE anywhere that
a suitable "variable" is in scope. The core already allows related
syntaxes in some utility commands, but not within DML commands.

I poked at this a little bit, and concluded that it's probably impossible
to do it exactly like that, at least not in a backward-compatible way.
The difficulty is that TYPE is an unreserved keyword, and can therefore
be a column name, while of course '%' is a valid operator. So for example

SELECT x::int%type FROM ...

currently means "cast column x to integer and then modulo it by column
type". AFAICS there's no way to introduce %TYPE into the :: cast syntax
without breaking this interpretation.

I suppose that we could dodge this ambiguity by allowing %TYPE only in the
CAST() notation, but this would be the first time that CAST and :: had any
differences in how the type name could be spelled, and that's not a nice
inconsistency to introduce either.

What's possibly more palatable is to introduce some other special notation
for "obtain the type of this expression at parse time". I'm thinking for
example about

SELECT x::pg_typeof(some_expression) FROM ...

Maybe it would be too confusing to overload "pg_typeof" this way,
in which case we could choose some other name. Aside from that
consideration, this approach would have the effect of preventing
"pg_typeof" from being used as an actual type name, or at least from
being used as a type name that can have typmod, but that doesn't seem
like a huge drawback.

This way would have the rather nice property that some_expression could
actually be any parseable expression, not merely a qualified variable
name (which I think is the only case that we'd have had any hope of
supporting with %TYPE).

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/29/2015 05:13 PM, Tom Lane wrote:

What's possibly more palatable is to introduce some other special
notation for "obtain the type of this expression at parse time".
I'm thinking for example about

SELECT x::pg_typeof(some_expression) FROM ...

Maybe it would be too confusing to overload "pg_typeof" this way,
in which case we could choose some other name. Aside from that
consideration, this approach would have the effect of preventing
"pg_typeof" from being used as an actual type name, or at least
from being used as a type name that can have typmod, but that
doesn't seem like a huge drawback.

This way would have the rather nice property that some_expression
could actually be any parseable expression, not merely a qualified
variable name (which I think is the only case that we'd have had
any hope of supporting with %TYPE).

You think this could be made to work?

SELECT x::TYPE OF(some_expression) FROM ...

Then we would also have:

SELECT CAST(x AS TYPE OF(some_expression)) FROM ...

Seems nicer than pg_typeof

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVuW3GAAoJEDfy90M199hlY6UP/026qCp5sxSz8zsnY9FyqqMp
Xf+a1nlqZFB821zQtMx7NtKtxjiC45jqbPSerLqCSbbpMftLG/iy5+/wdWJqAIoK
283Q23hD6r7hPwR2nown3BH5F1AFGCppG5KWebOgl01jVQSWBfFiRLrImFi/2FVs
sp3fHFmONp2kIYoAZwyFyZXv2n4D9Qhp0tIq94Dz0LGaszsfpYObCapPkgwAgaYZ
TSk5FAmD+IIJsNadhuk6IfJRObY5DgLL5keJSiuNs4Xpixq72KjqgnYQeXqgoT6U
AOqEkyg/KejkBSmuZRtHc9qnewGPzQn9TBXZEGPF+/ifpHC7+gL2au97kUW35j5l
3Ox57hTTRgr3oRvrEkGN/1uM/yDiobXb2HOp70mIeuYAWp3juwfxZQybRMYCoM8a
O/oyJqTSX2Dn/GkzeAOxbaQJulMeJfkivnwak0BhdaX3d4bDJz1ylNgz/B4Gtcnn
x4FVdTEfTBGFKFmfyDB5iAvpRrDCCDYcd2KcHA1J34vm8Ccm6m3aauJoi4zqsubh
bQnia2aoIAtnIOei/zVaST7+G+B3WAod3MDmrctjkx1lACTIeQVHq7q/A3iUPwcF
7Lfu9vr/6IBsAlvkdsX7zNZsIzAlov+ObrKZHBxeq2lB31MH1jeulX8lx+131+aI
ATx7kmeBlB60dkccMEhv
=VgLe
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Joe Conway <mail@joeconway.com> writes:

On 07/29/2015 05:13 PM, Tom Lane wrote:

What's possibly more palatable is to introduce some other special
notation for "obtain the type of this expression at parse time".
I'm thinking for example about

SELECT x::pg_typeof(some_expression) FROM ...

You think this could be made to work?

SELECT x::TYPE OF(some_expression) FROM ...

Hmmm ... that looks kind of nice, but a quick experiment with
bison says it's ambiguous. I tried this just as proof-of-concept:

*** src/backend/parser/gram.y~	Fri Jul 24 21:40:02 2015
--- src/backend/parser/gram.y	Wed Jul 29 22:45:04 2015
*************** GenericType:
*** 11065,11070 ****
--- 11065,11074 ----
  					$$->typmods = $3;
  					$$->location = @1;
  				}
+ 			| TYPE_P OF '(' a_expr ')'
+ 				{
+ 					$$ = makeTypeNameFromNameList(lcons(makeString($1), $2));
+ 				}
  		;

opt_type_modifiers: '(' expr_list ')' { $$ = $2; }

and got a shift/reduce conflict. I'm not quite sure why, but since OF
is also not a reserved keyword, it's likely that this is unfixable.
In fact, I also tried "TYPE_P FROM", not because that is especially
great syntax but because FROM *is* fully reserved, and that did not
work either. So this isn't looking like a promising line of thought.

We can definitely do

SELECT x::any_single_unreserved_word(some_expression) FROM ...

because that's actually not something the grammar needs to distinguish
from type-with-a-typmod; we can deal with the special case in
LookupTypeName. It's just a matter of picking a word people like.

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/29/2015 07:58 PM, Tom Lane wrote:

We can definitely do

SELECT x::any_single_unreserved_word(some_expression) FROM ...

because that's actually not something the grammar needs to
distinguish from type-with-a-typmod; we can deal with the special
case in LookupTypeName. It's just a matter of picking a word
people like.

What about just TYPE then?

SELECT x::TYPE(some_expression) FROM ...
SELECT CAST (x AS TYPE(some_expression)) FROM ...

- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVuar5AAoJEDfy90M199hlN8gP/i2IdtOsJ1PasKfUjAegbHf5
HIWLI7lZw2OMb451zrrNJbfTk1xY+OUJX8tRTLku8GyoZ9FrhDnBo0JuZuHMQOo4
ulWPH7JYGQVb89FYANNbubIehfJ0Y5TCr/ihkpmeVR6sTR3OZDSvdVtymF34wfZE
96i2S6QqWHN4V6hNXjTuzaIu4BXFXvZg3N9yNvBRrpnif53jfKPnca6wSeHJgTWv
w8L6mKQbLDW+5azVmuFX/1PyLxMRphsZL6G4+yyASkzQP2VOGDRQrM4Uavoot9Ja
l1Ez4bBoK3ERxfovnSWfwlsqhKmQ41TijoIu/Ex/s1O3dL2LVQ2qBp8cCl8pX9zq
Fnk11ueAvjkVt8/mIQFkGY+noes8vqWGe6yB0FYXjJvFfL4DXgfmthdyyCGJM1l9
JLI034tkflXKEkk5Ty9gOeAaMzqztqmIRYoQKK7O18DOKNH3Fgoa5Vh2Fz/iJI6G
rjQtfcZwv6ukN0qyQ8QB42CvLJVQ5KVwdTSr/93eCipSIuTPJNEoIBSh7H02WN7Q
fqQcKsM9m9ZTkAYP9uQCMEwusiKoPZt41Tdwf5fbhuOHoSim2Tab63eMEoUkRsqu
Bgqql/U5/MRsoAoDp4ALr2LbugnnTVNhrqrP58e45yl+694UEyh9XRpZmWUpX9Lw
k+qPyOJCnLBwOcmS0tv1
=+T37
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jul 30, 2015 at 12:41 AM, Joe Conway <mail@joeconway.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/29/2015 07:58 PM, Tom Lane wrote:

We can definitely do

SELECT x::any_single_unreserved_word(some_expression) FROM ...

because that's actually not something the grammar needs to
distinguish from type-with-a-typmod; we can deal with the special
case in LookupTypeName. It's just a matter of picking a word
people like.

What about just TYPE then?

SELECT x::TYPE(some_expression) FROM ...
SELECT CAST (x AS TYPE(some_expression)) FROM ...

The use case for dynamic column casting escapes me, so I don't feel
comfortable offering anything there. That discomfort will stop me for about
a paragraph.

As for record set casting, what about leveraging usage of the LIKE keyword
from derivative table creation, in some or all of these permutations:
a) SELECT * FROM recordset_returning_function() as t(LIKE
my_table_name);
b) SELECT * FROM recordset_returning_function() as t(LIKE
'my_table_name'::regclass);
c) SELECT * FROM recordset_returning_function() as t(LIKE
pg_typeof(p_some_anyelement_var));

Stepping into my discomfort zone, would the logical extension of allowing
the "b" case, where oids stand in for the type they reference, lead to:
SELECT CAST(x AS 'some_expression'::regclass) FROM ...

Joe Conway <mail@joeconway.com> writes:

What about just TYPE then?

SELECT x::TYPE(some_expression) FROM ...
SELECT CAST (x AS TYPE(some_expression)) FROM ...

Yeah, that would work. Quick-hack proof-of-concept patch attached.
Some usage examples in the regression database:

regression=# select pg_typeof(43::type(q1)) from int8_tbl;
pg_typeof
-----------
bigint
bigint
bigint
bigint
bigint
(5 rows)

regression=# select pg_typeof(43::type(q1/0.0)) from int8_tbl;
pg_typeof
-----------
numeric
numeric
numeric
numeric
numeric
(5 rows)

regression=# select pg_typeof(43::type(f1)) from point_tbl;
ERROR: cannot cast type integer to point
LINE 1: select pg_typeof(43::type(f1)) from point_tbl;
^

The main limitation of this patch is that it won't work for call sites
that pass pstate == NULL to LookupTypeName. There are a fair number
of them, some of which wouldn't care because they could never invoke
this notation anyway, but for others we'd need to do some work to cons
up a suitable pstate.

regards, tom lane

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

What about just TYPE then?

SELECT x::TYPE(some_expression) FROM ...
SELECT CAST (x AS TYPE(some_expression)) FROM ...

Yeah, that would work. Quick-hack proof-of-concept patch attached.

I'm amazed that this works without hacking the grammar itself, but in
retrospect that's expected.

+ 	else if (pstate != NULL &&
+ 			 list_length(typeName->typmods) == 1 &&
+ 			 list_length(typeName->names) == 1 &&
+ 			 strcmp(strVal(linitial(typeName->names)), "type") == 0)
+ 	{
+ 		/* TYPE(expression) notation */
+ 		Node	   *typexpr = (Node *) linitial(typeName->typmods);

This is a rather ugly, but I guess not untenable. I suppose we don't
care about any actual typmod, do we?. Will this be of any use with the
PostGIS types and such, for which the typmod is not merely a size limit?
Also INTERVAL has some funny typmod rules, not sure if that affects
usage of this construct.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Tom Lane wrote:

Yeah, that would work. Quick-hack proof-of-concept patch attached.

This is a rather ugly, but I guess not untenable. I suppose we don't
care about any actual typmod, do we?.

We get the type and typmod both from the expression. Example:

regression=# create table ref (f1 varchar, f2 varchar(3));
CREATE TABLE
regression=# insert into ref values('1','2');
INSERT 0 1
regression=# select '1234567890'::type(f1) from ref;
type
------------
1234567890
(1 row)

regression=# select '1234567890'::type(f2) from ref;
type
------
123
(1 row)

Will this be of any use with the
PostGIS types and such, for which the typmod is not merely a size limit?

Don't see why not. They still have to follow the rule that typmod is a
static property of an expression.

Also INTERVAL has some funny typmod rules, not sure if that affects
usage of this construct.

I would not think so. The weirdness about INTERVAL mainly has to do with
SQL's, ahem, inventive collection of ways to write an interval constant,
and that wouldn't really be an issue for any practical use of this
construct AFAICS. Whatever you write as the expression, we're going to
be able to reduce to a type OID and typmod.

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

On 07/30/2015 09:51 AM, Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

What about just TYPE then?

SELECT x::TYPE(some_expression) FROM ...
SELECT CAST (x AS TYPE(some_expression)) FROM ...

The main limitation of this patch is that it won't work for call sites
that pass pstate == NULL to LookupTypeName. There are a fair number
of them, some of which wouldn't care because they could never invoke
this notation anyway, but for others we'd need to do some work to cons
up a suitable pstate.

Sorry it took so long for me to get back to this, but any reason the
attached won't work?

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Joe Conway wrote:

On 07/30/2015 09:51 AM, Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

What about just TYPE then?

SELECT x::TYPE(some_expression) FROM ...
SELECT CAST (x AS TYPE(some_expression)) FROM ...

The main limitation of this patch is that it won't work for call sites
that pass pstate == NULL to LookupTypeName. There are a fair number
of them, some of which wouldn't care because they could never invoke
this notation anyway, but for others we'd need to do some work to cons
up a suitable pstate.

Sorry it took so long for me to get back to this, but any reason the
attached won't work?

So, is this going anywhere?

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Joe Conway wrote:

On 07/30/2015 09:51 AM, Tom Lane wrote:

The main limitation of this patch is that it won't work for call sites
that pass pstate == NULL to LookupTypeName. There are a fair number
of them, some of which wouldn't care because they could never invoke
this notation anyway, but for others we'd need to do some work to cons
up a suitable pstate.

Sorry it took so long for me to get back to this, but any reason the
attached won't work?

So, is this going anywhere?

Oh, sorry, was I on the hook to review that?

[ quick look... ] This doesn't seem like it responds to my criticism
above. I think what we want is that for every LookupTypeName call site
that could potentially be invoking this notation, we must actually make
provision for passing a valid pstate, one containing in particular the
source text for the nodetree we're parsing. Without that we will not
get error messages of the quality we expect (with error pointers).

Another issue now that I look at it is that parser-detected semantic
problems in the expression will result in ereport(ERROR), rather than
returning NULL which is what you'd kind of expect from the API spec for
LookupTypeName. That's probably all right considering that many other
syntactic issues throw errors inside this function; but maybe we'd better
adjust the API spec.

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

On 01/08/2016 07:31 AM, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

So, is this going anywhere?

Oh, sorry, was I on the hook to review that?

[ quick look... ] This doesn't seem like it responds to my criticism
above. I think what we want is that for every LookupTypeName call site
that could potentially be invoking this notation, we must actually make
provision for passing a valid pstate, one containing in particular the
source text for the nodetree we're parsing. Without that we will not
get error messages of the quality we expect (with error pointers).

Another issue now that I look at it is that parser-detected semantic
problems in the expression will result in ereport(ERROR), rather than
returning NULL which is what you'd kind of expect from the API spec for
LookupTypeName. That's probably all right considering that many other
syntactic issues throw errors inside this function; but maybe we'd better
adjust the API spec.

Ok, back to the drawing board. Thanks for the feedback.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Joe Conway wrote:

Ok, back to the drawing board. Thanks for the feedback.

Closing this one as returned-with-feedback. Please do resubmit for
CF 2016-03.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jan 28, 2016 at 6:12 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Joe Conway wrote:

Ok, back to the drawing board. Thanks for the feedback.

Closing this one as returned-with-feedback. Please do resubmit for
CF 2016-03.

Joe, it looks like you reactivated this patch for CF 2016-03 even
though it hasn't been updated. So, you want a review of an updated
patch you haven't written yet?

I think this should be marked Returned with Feedback and you can
resubmit for 9.7.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 03/11/2016 08:31 AM, Robert Haas wrote:

On Thu, Jan 28, 2016 at 6:12 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Joe Conway wrote:

Ok, back to the drawing board. Thanks for the feedback.

Closing this one as returned-with-feedback. Please do resubmit for
CF 2016-03.

Joe, it looks like you reactivated this patch for CF 2016-03 even
though it hasn't been updated. So, you want a review of an updated
patch you haven't written yet?

I think this should be marked Returned with Feedback and you can
resubmit for 9.7.

Understood. I was scrambling to get updates to a number of these prior
to the commitfest starting and could not manage to get to this one. Done.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

On Fri, Mar 11, 2016 at 8:44 AM, Joe Conway <mail@joeconway.com> wrote:

On 03/11/2016 08:31 AM, Robert Haas wrote:

On Thu, Jan 28, 2016 at 6:12 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Joe Conway wrote:

Ok, back to the drawing board. Thanks for the feedback.

Closing this one as returned-with-feedback. Please do resubmit for
CF 2016-03.

Joe, it looks like you reactivated this patch for CF 2016-03 even
though it hasn't been updated. So, you want a review of an updated
patch you haven't written yet?

I think this should be marked Returned with Feedback and you can
resubmit for 9.7.

Understood. I was scrambling to get updates to a number of these prior
to the commitfest starting and could not manage to get to this one. Done.

Thanks for understanding. Rejecting patches is not much more fun that
getting your patches rejected, but it's got to be done... sorry!

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 03/11/2016 08:51 AM, Robert Haas wrote:

Thanks for understanding. Rejecting patches is not much more fun that
getting your patches rejected, but it's got to be done... sorry!

+1

Sorry I didn't already do this myself when it became clear I wasn't
going to get it done in time.

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development