Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] dblink: add polymorphic functions.

[Robert Haas]

On Fri, Mar 11, 2016 at 8:44 AM, Joe Conway  wrote:
> On 03/11/2016 08:31 AM, Robert Haas wrote:
>> On Thu, Jan 28, 2016 at 6:12 AM, Alvaro Herrera
>>  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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

On 03/11/2016 08:31 AM, Robert Haas wrote:
> On Thu, Jan 28, 2016 at 6:12 AM, Alvaro Herrera
>  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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] dblink: add polymorphic functions.

[Robert Haas]

On Thu, Jan 28, 2016 at 6:12 AM, Alvaro Herrera
 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

Re: [HACKERS] dblink: add polymorphic functions.

[Alvaro Herrera]

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 Herrerahttp://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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

On 01/08/2016 07:31 AM, Tom Lane wrote:
> Alvaro Herrera  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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] dblink: add polymorphic functions.

[Alvaro Herrera]

Joe Conway wrote:
> On 07/30/2015 09:51 AM, Tom Lane wrote:
> > Joe Conway  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 Herrerahttp://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

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Alvaro Herrera  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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

On 07/30/2015 09:51 AM, Tom Lane wrote:
> Joe Conway  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
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 5b0d568..a1aba26 100644
*** a/src/backend/parser/parse_agg.c
--- b/src/backend/parser/parse_agg.c
*** check_agglevels_and_constraints(ParseSta
*** 485,490 
--- 485,493 
  err = _("grouping operations are not allowed in trigger WHEN conditions");
  
  			break;
+ 		case EXPR_KIND_TYPE:
+ 			/* okay */
+ 			break;
  
  			/*
  			 * There is intentionally no default: case here, so that the
*** transformWindowFuncCall(ParseState *psta
*** 842,847 
--- 845,853 
  		case EXPR_KIND_TRIGGER_WHEN:
  			err = _("window functions are not allowed in trigger WHEN conditions");
  			break;
+ 		case EXPR_KIND_TYPE:
+ 			/* okay */
+ 			break;
  
  			/*
  			 * There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index fa77ef1..9f76ee6 100644
*** a/src/backend/parser/parse_expr.c
--- b/src/backend/parser/parse_expr.c
*** transformSubLink(ParseState *pstate, Sub
*** 1690,1695 
--- 1690,1696 
  		case EXPR_KIND_OFFSET:
  		case EXPR_KIND_RETURNING:
  		case EXPR_KIND_VALUES:
+ 		case EXPR_KIND_TYPE:
  			/* okay */
  			break;
  		case EXPR_KIND_CHECK_CONSTRAINT:
*** ParseExprKindName(ParseExprKind exprKind
*** 3225,3230 
--- 3226,3233 
  			return "EXECUTE";
  		case EXPR_KIND_TRIGGER_WHEN:
  			return "WHEN";
+ 		case EXPR_KIND_TYPE:
+ 			return "TYPE";
  
  			/*
  			 * There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index 6616639..19a2684 100644
*** a/src/backend/parser/parse_type.c
--- b/src/backend/parser/parse_type.c
***
*** 19,25 
--- 19,27 
  #include "catalog/pg_type.h"
  #include "lib/stringinfo.h"
  #include "nodes/makefuncs.h"
+ #include "nodes/nodeFuncs.h"
  #include "parser/parser.h"
+ #include "parser/parse_expr.h"
  #include "parser/parse_type.h"
  #include "utils/array.h"
  #include "utils/builtins.h"
*** static int32 typenameTypeMod(ParseState
*** 52,58 
   * found but is a shell, and there is typmod decoration, an error will be
   * thrown --- this is intentional.
   *
!  * pstate is only used for error location info, and may be NULL.
   */
  Type
  LookupTypeName(ParseState *pstate, const TypeName *typeName,
--- 54,60 
   * found but is a shell, and there is typmod decoration, an error will be
   * thrown --- this is intentional.
   *
!  * In most cases pstate is only used for error location info, and may be NULL.
   */
  Type
  LookupTypeName(ParseState *pstate, const TypeName *typeName,
*** LookupTypeName(ParseState *pstate, const
*** 60,66 
  {
  	Oid			typoid;
  	HeapTuple	tup;
! 	int32		typmod;
  
  	if (typeName->names == NIL)
  	{
--- 62,68 
  {
  	Oid			typoid;
  	HeapTuple	tup;
! 	int32		typmod = -2;
  
  	if (typeName->names == NIL)
  	{
*** LookupTypeName(ParseState *pstate, const
*** 143,148 
--- 145,172 
  			format_type_be(typoid;
  		}
  	}
+ 	else if (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);
+ 
+ 		/* If needed, create a dummy ParseState for transformExpr */
+ 		if (pstate == NULL)
+ 			pstate = make_parsestate(NULL);
+ 
+ 		typexpr = transformExpr(pstate, typexpr, EXPR_KIND_TYPE);
+ 
+ 		/* We needn't bother assigning collations to the expr */
+ 		/* We use the expression's type/typmod and then throw the expr away */
+ 		typoid = exprType(typexpr);
+ 		typmod = exprTypmod(typexpr);
+ 
+ 		/* If an array reference, return the array type instead */
+ 		if (typeName->arrayBounds != NIL)
+ 			typoid = get_array_type(typoid);
+ 	}
  	else
  	{
  		/* Normal reference to a type name */
*** LookupTypeName(ParseState *pstate, const
*** 192,198 
  	if (!HeapTupleIsValid(tup)) /* should not happen */
  		elog(ERROR, "cache lookup failed for type %u", typoid);
  
! 	typmod = typenameTypeMod(pstate, typeName, 

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Joe Conway m...@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

diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index 6616639..d5d0f73 100644
*** a/src/backend/parser/parse_type.c
--- b/src/backend/parser/parse_type.c
***
*** 19,25 
--- 19,27 
  #include catalog/pg_type.h
  #include lib/stringinfo.h
  #include nodes/makefuncs.h
+ #include nodes/nodeFuncs.h
  #include parser/parser.h
+ #include parser/parse_expr.h
  #include parser/parse_type.h
  #include utils/array.h
  #include utils/builtins.h
*** static int32 typenameTypeMod(ParseState 
*** 52,58 
   * found but is a shell, and there is typmod decoration, an error will be
   * thrown --- this is intentional.
   *
!  * pstate is only used for error location info, and may be NULL.
   */
  Type
  LookupTypeName(ParseState *pstate, const TypeName *typeName,
--- 54,61 
   * found but is a shell, and there is typmod decoration, an error will be
   * thrown --- this is intentional.
   *
!  * In most cases pstate is only used for error location info, and may be NULL.
!  * However, the TYPE(expression) syntax is not accepted when pstate is NULL.
   */
  Type
  LookupTypeName(ParseState *pstate, const TypeName *typeName,
*** LookupTypeName(ParseState *pstate, const
*** 143,148 
--- 146,188 
  			format_type_be(typoid;
  		}
  	}
+ 	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);
+ 
+ 		/* XXX should invent a new EXPR_KIND for this, likely */
+ 		typexpr = transformExpr(pstate, typexpr, EXPR_KIND_SELECT_TARGET);
+ 
+ 		/* We needn't bother assigning collations to the expr */
+ 
+ 		/* We use the expression's type/typmod and then throw the expr away */
+ 		typoid = exprType(typexpr);
+ 
+ 		/* If an array reference, return the array type instead */
+ 		if (typeName-arrayBounds != NIL)
+ 			typoid = get_array_type(typoid);
+ 
+ 		if (!OidIsValid(typoid))
+ 		{
+ 			if (typmod_p)
+ *typmod_p = -1;
+ 			return NULL;
+ 		}
+ 
+ 		if (typmod_p)
+ 			*typmod_p = exprTypmod(typexpr);
+ 
+ 		/* Duplicative, but I'm too lazy to refactor this function right now */
+ 		tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typoid));
+ 		if (!HeapTupleIsValid(tup)) /* should not happen */
+ 			elog(ERROR, cache lookup failed for type %u, typoid);
+ 
+ 		return (Type) tup;
+ 	}
  	else
  	{
  		/* Normal reference to a type name */

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

Re: [HACKERS] dblink: add polymorphic functions.

[Alvaro Herrera]

Tom Lane wrote:
 Joe Conway m...@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 Herrerahttp://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

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Alvaro Herrera alvhe...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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 t...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

On Wed, Jul 29, 2015 at 12:14 PM, Joe Conway m...@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 t...@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.

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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

 Corey Huinker corey.huin...@gmail.com writes:
  On Wed, Jul 29, 2015 at 3:48 AM, Heikki Linnakangas hlinn...@iki.fi
 wrote:
  Let's pursue the CAST(srf() AS row_rtype) syntax that Joe suggested
  upthread (
  http://www.postgresql.org/message-id/559a9643.9070...@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.

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Corey Huinker corey.huin...@gmail.com writes:
 On Wed, Jul 29, 2015 at 10:48 AM, Tom Lane t...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Merlin Moncure]

On Wed, Jul 29, 2015 at 12:53 PM, Joe Conway m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Merlin Moncure mmonc...@gmail.com writes:
 On Wed, Jul 29, 2015 at 12:53 PM, Joe Conway m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

 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.

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Joe Conway m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Heikki Linnakangas]

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 
(http://www.postgresql.org/message-id/559a9643.9070...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Corey Huinker corey.huin...@gmail.com writes:
 On Wed, Jul 29, 2015 at 3:48 AM, Heikki Linnakangas hlinn...@iki.fi wrote:
 Let's pursue the CAST(srf() AS row_rtype) syntax that Joe suggested
 upthread (
 http://www.postgresql.org/message-id/559a9643.9070...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

On Wed, Jul 29, 2015 at 3:48 AM, Heikki Linnakangas hlinn...@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 (
 http://www.postgresql.org/message-id/559a9643.9070...@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.

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

On Wed, Jul 8, 2015 at 12:21 PM, Joe Conway m...@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.

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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 m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Merlin Moncure]

On Wed, Jul 8, 2015 at 10:12 AM, Joe Conway m...@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 m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Pavel Stehule]

2015-07-08 17:12 GMT+02:00 Joe Conway m...@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 m...@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


 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-

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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

 On Wed, Jul 8, 2015 at 10:12 AM, Joe Conway m...@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 m...@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.

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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

Re: [HACKERS] dblink: add polymorphic functions.

[Michael Paquier]

On Mon, Jul 6, 2015 at 11:52 PM, Joe Conway m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Michael Paquier]

On Mon, Jul 6, 2015 at 4:23 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Mon, Jul 6, 2015 at 10:00 AM, Joe Conway m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Merlin Moncure]

On Thu, Feb 19, 2015 at 4:06 PM, Corey Huinker corey.huin...@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
 frompg_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,','))
 fromx;

 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

Re: [HACKERS] dblink: add polymorphic functions.

[Merlin Moncure]

On Mon, Jul 6, 2015 at 11:13 AM, Corey Huinker corey.huin...@gmail.com wrote:
 On Mon, Jul 6, 2015 at 11:33 AM, Merlin Moncure mmonc...@gmail.com wrote:

 On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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

 On Mon, Jul 6, 2015 at 11:13 AM, Corey Huinker corey.huin...@gmail.com
 wrote:
  On Mon, Jul 6, 2015 at 11:33 AM, Merlin Moncure mmonc...@gmail.com
 wrote:
 
  On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway m...@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 

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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

 On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway m...@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.

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

On Sun, Jul 5, 2015 at 9:00 PM, Joe Conway m...@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.

Re: [HACKERS] dblink: add polymorphic functions.

[Michael Paquier]

On Mon, Jul 6, 2015 at 4:25 AM, Joe Conway m...@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
diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile
index b8d5157..5189758 100644
--- a/contrib/dblink/Makefile
+++ b/contrib/dblink/Makefile
@@ -6,7 +6,8 @@ PG_CPPFLAGS = -I$(libpq_srcdir)
 SHLIB_LINK = $(libpq)
 
 EXTENSION = dblink
-DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql
+DATA = dblink--1.2.sql dblink--1.1--1.2.sql dblink--1.0--1.1.sql \
+	dblink--unpackaged--1.0.sql
 PGFILEDESC = dblink - connect to other PostgreSQL databases
 
 REGRESS = paths dblink
diff --git a/contrib/dblink/dblink--1.1--1.2.sql b/contrib/dblink/dblink--1.1--1.2.sql
new file mode 100644
index 000..a82e507
--- /dev/null
+++ b/contrib/dblink/dblink--1.1--1.2.sql
@@ -0,0 +1,44 @@
+/* contrib/dblink/dblink--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use ALTER EXTENSION dblink UPDATE TO '1.2' to load this file. \quit
+
+CREATE FUNCTION dblink (text, text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_get_result (text, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_get_result (text, bool, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_fetch (text, int, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_fetch (text, int, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
diff --git a/contrib/dblink/dblink--1.1.sql b/contrib/dblink/dblink--1.2.sql
similarity index 84%
rename from contrib/dblink/dblink--1.1.sql
rename to contrib/dblink/dblink--1.2.sql
index 8733553..1d84df2 100644
--- a/contrib/dblink/dblink--1.1.sql
+++ b/contrib/dblink/dblink--1.2.sql
@@ -1,4 +1,4 @@
-/* contrib/dblink/dblink--1.1.sql */
+/* contrib/dblink/dblink--1.2.sql */
 
 -- complain if script is sourced in psql, rather than via CREATE EXTENSION
 \echo Use CREATE EXTENSION dblink to load this file. \quit
@@ -71,6 +71,16 @@ RETURNS setof record
 AS 'MODULE_PATHNAME','dblink_fetch'
 LANGUAGE C STRICT;
 
+CREATE FUNCTION dblink_fetch (text, int, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_fetch (text, int, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
+
 CREATE FUNCTION dblink_fetch (text, text, int)
 RETURNS setof record
 AS 'MODULE_PATHNAME','dblink_fetch'
@@ -121,6 +131,26 @@ RETURNS setof record
 AS 'MODULE_PATHNAME','dblink_record'
 LANGUAGE C STRICT;
 
+CREATE FUNCTION dblink (text, text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
 CREATE FUNCTION dblink_exec 

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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-
inti;
intnumargs;
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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

On Mon, Jul 6, 2015 at 10:08 AM, Joe Conway m...@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-
 inti;
 intnumargs;
 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.

Re: [HACKERS] dblink: add polymorphic functions.

[Merlin Moncure]

On Mon, Jul 6, 2015 at 9:52 AM, Joe Conway m...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Merlin Moncure]

On Mon, Jul 6, 2015 at 9:35 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Joe Conway m...@joeconway.com writes:
 Actually, I had in mind something like:
 8-
 inti;
 intnumargs;
 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

Re: [HACKERS] dblink: add polymorphic functions.

[Tom Lane]

Joe Conway m...@joeconway.com writes:
 Actually, I had in mind something like:
 8-
 inti;
 intnumargs;
 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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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-
diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile
index b8d5157..f31b9f7 100644
*** a/contrib/dblink/Makefile
--- b/contrib/dblink/Makefile
*** PG_CPPFLAGS = -I$(libpq_srcdir)
*** 6,12 
  SHLIB_LINK = $(libpq)
  
  EXTENSION = dblink
! DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql
  PGFILEDESC = dblink - connect to other PostgreSQL databases
  
  REGRESS = paths dblink
--- 6,13 
  SHLIB_LINK = $(libpq)
  
  EXTENSION = dblink
! DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql \
!dblink--1.2.sql dblink--1.1--1.2.sql
  PGFILEDESC = dblink - connect to other PostgreSQL databases
  
  REGRESS = paths dblink
diff --git a/contrib/dblink/dblink--1.1--1.2.sql b/contrib/dblink/dblink--1.1--1.2.sql
index ...128611d .
*** a/contrib/dblink/dblink--1.1--1.2.sql
--- b/contrib/dblink/dblink--1.1--1.2.sql
***
*** 0 
--- 1,46 
+ /* contrib/dblink/dblink--1.1--1.2.sql */
+ 
+ -- complain if script is sourced in psql, rather than via ALTER EXTENSION
+ \echo Use ALTER EXTENSION dblink UPDATE TO '1.2' to load this file. \quit
+ 
+ CREATE FUNCTION dblink (text, text, anyelement)
+ RETURNS setof anyelement
+ AS 'MODULE_PATHNAME','dblink_record'
+ LANGUAGE C;
+ 
+ CREATE FUNCTION dblink (text, text, boolean, anyelement)
+ RETURNS setof anyelement
+ AS 'MODULE_PATHNAME','dblink_record'
+ LANGUAGE C;
+ 
+ CREATE FUNCTION dblink (text, anyelement)
+ RETURNS setof anyelement
+ AS 'MODULE_PATHNAME','dblink_record'
+ LANGUAGE C;
+ 
+ CREATE FUNCTION dblink (text, boolean, anyelement)
+ RETURNS setof anyelement
+ AS 'MODULE_PATHNAME','dblink_record'
+ LANGUAGE C;
+ 
+ CREATE FUNCTION dblink_get_result(text, anyelement)
+ RETURNS SETOF anyelement
+ AS 'MODULE_PATHNAME', 'dblink_get_result'
+ LANGUAGE C;
+ 
+ CREATE FUNCTION dblink_get_result(text, bool, anyelement)
+ RETURNS SETOF anyelement
+ AS 'MODULE_PATHNAME', 'dblink_get_result'
+ LANGUAGE C;
+ 
+ CREATE FUNCTION dblink_fetch (text, int, anyelement)
+ RETURNS setof anyelement
+ AS 'MODULE_PATHNAME','dblink_fetch'
+ LANGUAGE C;
+ 
+ CREATE FUNCTION dblink_fetch (text, int, boolean, anyelement)
+ RETURNS setof anyelement
+ AS 'MODULE_PATHNAME','dblink_fetch'
+ LANGUAGE C;
+ 
+ 
diff --git a/contrib/dblink/dblink--1.2.sql b/contrib/dblink/dblink--1.2.sql
index ...1d84df2 .
*** a/contrib/dblink/dblink--1.2.sql
--- b/contrib/dblink/dblink--1.2.sql
***
*** 0 
--- 1,275 
+ /* contrib/dblink/dblink--1.2.sql */
+ 
+ -- complain if script is sourced in psql, rather than via CREATE EXTENSION
+ \echo Use CREATE EXTENSION dblink to load this file. \quit
+ 
+ -- dblink_connect now restricts non-superusers to password
+ -- authenticated connections
+ CREATE FUNCTION dblink_connect (text)
+ RETURNS text
+ AS 'MODULE_PATHNAME','dblink_connect'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION dblink_connect (text, text)
+ RETURNS text
+ AS 'MODULE_PATHNAME','dblink_connect'
+ LANGUAGE C STRICT;
+ 
+ -- dblink_connect_u allows non-superusers to use
+ -- non-password authenticated connections, but initially
+ -- privileges are revoked from public
+ CREATE FUNCTION dblink_connect_u (text)
+ RETURNS text
+ AS 'MODULE_PATHNAME','dblink_connect'
+ LANGUAGE C STRICT SECURITY DEFINER;
+ 
+ CREATE FUNCTION dblink_connect_u (text, text)
+ RETURNS text
+ AS 'MODULE_PATHNAME','dblink_connect'
+ LANGUAGE C STRICT SECURITY DEFINER;
+ 
+ REVOKE ALL ON FUNCTION dblink_connect_u (text) FROM public;
+ REVOKE ALL ON FUNCTION dblink_connect_u (text, text) FROM public;
+ 
+ CREATE FUNCTION dblink_disconnect ()
+ RETURNS text
+ AS 'MODULE_PATHNAME','dblink_disconnect'
+ LANGUAGE C 

Re: [HACKERS] dblink: add polymorphic functions - review

[Joe Conway]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 06/18/2015 09:57 PM, Pavel Stehule wrote:
 2015-06-19 6:32 GMT+02:00 Thomas Munro
 thomas.mu...@enterprisedb.com 
 mailto:thomas.mu...@enterprisedb.com:
 
 On Fri, Jun 19, 2015 at 4:17 PM, Pavel Stehule 
 pavel.steh...@gmail.com mailto:pavel.steh...@gmail.com wrote:
 I am sorry. I didn't the original mail in my mailbox.
 
 There is a (rather well hidden) way to ask Majordomo to send you a 
 message that arrived before you subscribed, so that you can reply 
 without creating a new thread.  I think you have to subscribe first
 so that you can log into it.  If you start here and browse to the
 message in the archive, you'll see a 'Mail this message to ...'
 link at the bottom:
 
 https://lists.postgresql.org/mj/mj_wwwusr/domain=postgresql.org
 
 I didn't know it. Thank you

Nice tip -- thanks! The patch was missing two new files - I posted a
complete patch using the original thread.


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

iQIcBAEBAgAGBQJVmYWDAAoJEDfy90M199hlT7YP/1rUX6TTJcTT45bRPWagbPSa
VS6NBihFDA4Kc6IDfvr7iS3nyWaHC1TDBrkhHKUfH1HSxXN492GkLCjG95HCSEHQ
AFylW3IarVpMdCxe7qJ2lH1CG69v8pabfmVPleymNZqz6j3KlNDXML9OqYjv+Aqx
G7Z6ciKC673+wQN6g4u9i8bbgJwHmqCdwEGLKgGqNnwErpNyTpxpr3tHZrkAC91B
KY6MBrNkpKyFbhxl5+1CIU4q6CWcYMtQSQ5T8eqKk2sntdHc3iDtrMFeSkK3OBRy
vbfaYrHRpFfk3OnBHNDlmzV0N8zVnGHrSgIkqA2tbFJ9ZMhT1AJavCrjBNeuiuMN
WVzpj+KY7lwbLI4xl3w+EUyqpnheVkNLuEit87EzIxiA2Sus1+XLFpPzE6O9pUN8
olN452ZYP5cjlApKxiEn0Bo0aEe7a+VpUja4/Z6dq3A7V42ci/HKNLwiI2n2iQQn
AiKM91Y3ir3FEcBg67CIbZdjxrrbwrzsMSfRi06INQIKTswMIlDR2A+2wVEtrDe5
kjqgVA1+bXfjlN2lfN9bp5KArXnvRLg+WA9Jj7SLB0I3QbiDgBqC7DS2ght0FxHp
4bY7hIL+kE8dc8vPs8C83WC8LYXxqFzi9vgZDk5kyqNTrQfLMz5YbPDiCQPUybJf
u9nNrqPmS+0BKOSr5o36
=orOZ
-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

Re: [HACKERS] dblink: add polymorphic functions.

[Joe Conway]

-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

[HACKERS] dblink: add polymorphic functions - review

[Pavel Stehule]

Hi

I am trying to check your patch - but diffs that you sent to mailing list
are inconsistent.

Please, can you send one complete patch?

Regards

Pavel

Re: [HACKERS] dblink: add polymorphic functions - review

[Pavel Stehule]

2015-06-19 6:32 GMT+02:00 Thomas Munro thomas.mu...@enterprisedb.com:

 On Fri, Jun 19, 2015 at 4:17 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
  I am sorry. I didn't the original mail in my mailbox.

 There is a (rather well hidden) way to ask Majordomo to send you a
 message that arrived before you subscribed, so that you can reply
 without creating a new thread.  I think you have to subscribe first so
 that you can log into it.  If you start here and browse to the message
 in the archive, you'll see a 'Mail this message to ...' link at the
 bottom:

 https://lists.postgresql.org/mj/mj_wwwusr/domain=postgresql.org


I didn't know it. Thank you

Regards

Pavel


 --
 Thomas Munro
 http://www.enterprisedb.com

Re: [HACKERS] dblink: add polymorphic functions - review

[Thomas Munro]

On Fri, Jun 19, 2015 at 4:17 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 I am sorry. I didn't the original mail in my mailbox.

There is a (rather well hidden) way to ask Majordomo to send you a
message that arrived before you subscribed, so that you can reply
without creating a new thread.  I think you have to subscribe first so
that you can log into it.  If you start here and browse to the message
in the archive, you'll see a 'Mail this message to ...' link at the
bottom:

https://lists.postgresql.org/mj/mj_wwwusr/domain=postgresql.org

-- 
Thomas Munro
http://www.enterprisedb.com


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

Re: [HACKERS] dblink: add polymorphic functions - review

[Pavel Stehule]

2015-06-19 2:17 GMT+02:00 Michael Paquier michael.paqu...@gmail.com:

 On Thu, Jun 18, 2015 at 10:44 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
  Hi
 
  I am trying to check your patch - but diffs that you sent to mailing list
  are inconsistent.
 
  Please, can you send one complete patch?

 In order to follow more easily the thread of this patch, could you
 answer directly to the existing thread? FWIW, it is here:

 http://www.postgresql.org/message-id/flat/CADkLM=d9AEZYQ2TpzOJQwBb42nV49YQy6b6S=z4q9svjiql...@mail.gmail.com#CADkLM=d9AEZYQ2TpzOJQwBb42nV49YQy6b6S=z4q9svjiql...@mail.gmail.com

 This will make the life of the future CFM(s) easier.
 Regards,


I am sorry. I didn't the original mail in my mailbox.

Regards

Pavel


 --
 Michael

Re: [HACKERS] dblink: add polymorphic functions - review

[Michael Paquier]

On Thu, Jun 18, 2015 at 10:44 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 Hi

 I am trying to check your patch - but diffs that you sent to mailing list
 are inconsistent.

 Please, can you send one complete patch?

In order to follow more easily the thread of this patch, could you
answer directly to the existing thread? FWIW, it is here:
http://www.postgresql.org/message-id/flat/CADkLM=d9AEZYQ2TpzOJQwBb42nV49YQy6b6S=z4q9svjiql...@mail.gmail.com#CADkLM=d9AEZYQ2TpzOJQwBb42nV49YQy6b6S=z4q9svjiql...@mail.gmail.com

This will make the life of the future CFM(s) easier.
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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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.huin...@gmail.com
wrote:

 nevermind. Found it.

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

 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.paqu...@gmail.com wrote:

 On Mon, Feb 23, 2015 at 12:03 PM, Corey Huinker corey.huin...@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




diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile
index e833b92..a00466e 100644
--- a/contrib/dblink/Makefile
+++ b/contrib/dblink/Makefile
@@ -6,7 +6,8 @@ PG_CPPFLAGS = -I$(libpq_srcdir)
 SHLIB_LINK = $(libpq)
 
 EXTENSION = dblink
-DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql
+DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql \
+   dblink--1.2.sql dblink--1.1--1.2.sql
 
 REGRESS = paths dblink
 REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress \
diff --git a/contrib/dblink/dblink--1.1--1.2.sql 
b/contrib/dblink/dblink--1.1--1.2.sql
index e5a0900..128611d 100644
--- a/contrib/dblink/dblink--1.1--1.2.sql
+++ b/contrib/dblink/dblink--1.1--1.2.sql
@@ -32,3 +32,15 @@ CREATE FUNCTION dblink_get_result(text, bool, anyelement)
 RETURNS SETOF anyelement
 AS 'MODULE_PATHNAME', 'dblink_get_result'
 LANGUAGE C;
+
+CREATE FUNCTION dblink_fetch (text, int, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_fetch (text, int, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
+
+
diff --git a/contrib/dblink/dblink--1.2.sql b/contrib/dblink/dblink--1.2.sql
index bf5ddaa..9d31e2e 100644
--- a/contrib/dblink/dblink--1.2.sql
+++ b/contrib/dblink/dblink--1.2.sql
@@ -71,6 +71,19 @@ RETURNS setof record
 AS 'MODULE_PATHNAME','dblink_fetch'
 LANGUAGE C STRICT;
 
+CREATE FUNCTION dblink_fetch (text, int, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_fetch (text, int, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_fetch'
+LANGUAGE C;
+
+
+
+
 CREATE FUNCTION dblink_fetch (text, text, int)
 RETURNS setof record
 AS 'MODULE_PATHNAME','dblink_fetch'
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 009b877..fde750c 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -537,52 +537,39 @@ dblink_fetch(PG_FUNCTION_ARGS)
char   *curname = NULL;
int howmany = 0;
boolfail = true;/* default to backward compatible */
+   int first_optarg;
 
prepTuplestoreResult(fcinfo);
 
DBLINK_INIT;
 
-   if (PG_NARGS() == 4)
+   if (get_fn_expr_argtype(fcinfo-flinfo,1) == TEXTOID)
{
-   /* text,text,int,bool */
+   /* text,text,int,[bool],[anytype] */
conname = text_to_cstring(PG_GETARG_TEXT_PP(0));
curname = text_to_cstring(PG_GETARG_TEXT_PP(1));
howmany = PG_GETARG_INT32(2);
-   fail = PG_GETARG_BOOL(3);
-
+   first_optarg = 3;
rconn = getConnectionByName(conname);
if (rconn)
conn = rconn-conn;
}
-   else if (PG_NARGS() == 3)
-   {
-   /* text,text,int or text,int,bool */
-   if (get_fn_expr_argtype(fcinfo-flinfo, 2) == BOOLOID)
-   {
-   curname = text_to_cstring(PG_GETARG_TEXT_PP(0));
-   howmany = PG_GETARG_INT32(1);
-   fail = PG_GETARG_BOOL(2);
-   conn = pconn-conn;
-   }
-   else
-   {
-   conname = text_to_cstring(PG_GETARG_TEXT_PP(0));
-   curname = text_to_cstring(PG_GETARG_TEXT_PP(1));
-   howmany = PG_GETARG_INT32(2);
-
-   rconn = getConnectionByName(conname);
-   if (rconn)
-   conn = rconn-conn;
-   }
-   }
-   else if (PG_NARGS() == 2)
+   else
{
-   /* text,int */
+   /* 

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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.paqu...@gmail.com
 wrote:

 On Mon, Feb 23, 2015 at 12:03 PM, Corey Huinker corey.huin...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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.paqu...@gmail.com
wrote:

 On Fri, Feb 20, 2015 at 2:50 PM, Corey Huinker corey.huin...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

nevermind. Found it.

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

 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.paqu...@gmail.com wrote:

 On Mon, Feb 23, 2015 at 12:03 PM, Corey Huinker corey.huin...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Michael Paquier]

On Mon, Feb 23, 2015 at 12:03 PM, Corey Huinker corey.huin...@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

[HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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
frompg_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,','))
fromx;

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)

[local]:ubuntu@dblink_test# *select * from
dblink('dbname=dblink_test','select * from pg_tables order by tablename
limit 2',null::pg_tables);*
 schemaname |  tablename   | tableowner | tablespace | hasindexes |
hasrules | hastriggers
+--++++--+-
 pg_catalog | pg_aggregate | postgres   || t  | f
 | f
 pg_catalog | pg_am| postgres   || t  | f
 | f
(2 rows)

Time: 6.813 ms



Obviously, this is a trivial case, but it shows that the polymorphic
function works as expected, and the code that uses it will be a lot more
straightforward.

Proposed patch attached.
diff --git a/contrib/dblink/dblink--1.1.sql b/contrib/dblink/dblink--1.1.sql
index 8733553..bf5ddaa 100644
--- a/contrib/dblink/dblink--1.1.sql
+++ b/contrib/dblink/dblink--1.1.sql
@@ -121,6 +121,26 @@ RETURNS setof record
 AS 'MODULE_PATHNAME','dblink_record'
 LANGUAGE C STRICT;
 
+CREATE FUNCTION dblink (text, text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
 CREATE FUNCTION dblink_exec (text, text)
 RETURNS text
 AS 'MODULE_PATHNAME','dblink_exec'
@@ -188,6 +208,16 @@ RETURNS SETOF record
 AS 'MODULE_PATHNAME', 'dblink_get_result'
 LANGUAGE C STRICT;
 
+CREATE FUNCTION dblink_get_result(text, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_get_result(text, bool, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
 CREATE FUNCTION dblink_get_connections()
 RETURNS text[]
 AS 'MODULE_PATHNAME', 'dblink_get_connections'
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 9fe750e..eb7f5f9 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -680,27 +680,68 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool 
is_async)
 
if (!is_async)
{
-   if (PG_NARGS() == 3)
+

Re: [HACKERS] dblink: add polymorphic functions.

[Michael Paquier]

On Fri, Feb 20, 2015 at 2:50 PM, Corey Huinker corey.huin...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

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.paqu...@gmail.com
 wrote:

 On Fri, Feb 20, 2015 at 7:06 AM, Corey Huinker corey.huin...@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

Re: [HACKERS] dblink: add polymorphic functions.

[Michael Paquier]

On Fri, Feb 20, 2015 at 7:06 AM, Corey Huinker corey.huin...@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