pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #14706: Dependencies not recorded properly for base types
15+ messages / 4 participants
[nested] [flat]
* BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-14 23:22 khuddleston@pivotal.io
0 siblings, 1 reply; 15+ messages in thread
From: khuddleston@pivotal.io @ 2017-06-14 23:22 UTC (permalink / raw)
To: pgsql-bugs
The following bug has been logged on the website:
Bug reference: 14706
Logged by: Karen Huddleston
Email address: khuddleston@pivotal.io
PostgreSQL version: 9.5.7
Operating system: macOS Sierra
Description:
Repro
```CREATE FUNCTION base_fn_in(cstring) returns opaque as 'boolin' language
internal;
CREATE FUNCTION base_fn_out(opaque) returns opaque as 'boolout' language
internal;
CREATE TYPE base_type(input=base_fn_in, output=base_fn_out);
\df
List of functions
Schema | Name | Result data type | Argument data types | Type
--------+--------------+------------------+---------------------+--------
public | base_fn_in | base_type | cstring | normal
public | base_fn_out | cstring | base_type | normal
DROP TYPE base_type;
\df
ERROR: cache lookup failed for type 22241 (format_type.c:137)
```
When I first create the functions, they are not dependent on other objects.
However, when I create base_type, the functions are modified to use the new
type. Then, dropping the type first causes the functions to have a cache
lookup error because they are missing a new dependency. It is possible to
clean up the state if you run DROP FUNCTION base_fn_out(base_type) CASCADE;
and then DROP FUNCTION base_fn_in(cstring);, but if you run DROP TYPE
base_type; first, it is impossible to drop base_fn_out. Base types should
make users run CASCADE to clean up dependencies in the other direction.
Additionally, the documentation claims that running DROP TYPE ... CASCADE
"Automatically drops objects that depend on the type (such as table columns,
functions, operators).", but it doesn't seem to do anything.
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 00:42 Tom Lane <tgl@sss.pgh.pa.us>
parent: khuddleston@pivotal.io
0 siblings, 2 replies; 15+ messages in thread
From: Tom Lane @ 2017-06-15 00:42 UTC (permalink / raw)
To: khuddleston@pivotal.io; +Cc: pgsql-bugs
khuddleston@pivotal.io writes:
> CREATE FUNCTION base_fn_in(cstring) returns opaque as 'boolin' language
> internal;
> CREATE FUNCTION base_fn_out(opaque) returns opaque as 'boolout' language
> internal;
> CREATE TYPE base_type(input=base_fn_in, output=base_fn_out);
You realize of course that this is long-deprecated style.
> DROP TYPE base_type;
> \df
> ERROR: cache lookup failed for type 22241 (format_type.c:137)
... but yeah, as long as we're supporting it at all, that shouldn't
happen. Thanks for the report!
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 01:02 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 15+ messages in thread
From: Tom Lane @ 2017-06-15 01:02 UTC (permalink / raw)
To: Karen Huddleston <khuddleston@pivotal.io>; +Cc: pgsql-bugs
Karen Huddleston <khuddleston@pivotal.io> writes:
> I didn't actually realize it was deprecated style. I'm not super familiar
> with the syntax for creating types and functions. Which part is deprecated
> and what should it be instead?
The way you're supposed to make a new type nowadays is
CREATE TYPE base_type; -- make a shell type
CREATE FUNCTION base_fn_in(cstring) returns base_type as ...
CREATE FUNCTION base_fn_out(base_type) returns cstring as ...
-- convert shell to real type
CREATE TYPE base_type(input=base_fn_in, output=base_fn_out);
In this way the function signatures are legal from the get-go,
and the dependencies are right too. The business with "opaque"
as a placeholder has been deprecated for circa 15 years; did
you find that example in any modern documentation?
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 04:52 Michael Paquier <michael.paquier@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2017-06-15 04:52 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs
On Thu, Jun 15, 2017 at 9:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> khuddleston@pivotal.io writes:
>> CREATE FUNCTION base_fn_in(cstring) returns opaque as 'boolin' language
>> internal;
>> CREATE FUNCTION base_fn_out(opaque) returns opaque as 'boolout' language
>> internal;
>> CREATE TYPE base_type(input=base_fn_in, output=base_fn_out);
>
> You realize of course that this is long-deprecated style.
>
>> DROP TYPE base_type;
>> \df
>> ERROR: cache lookup failed for type 22241 (format_type.c:137)
>
> ... but yeah, as long as we're supporting it at all, that shouldn't
> happen. Thanks for the report!
The return type of the input function and argument type of the output
function get updated when the type is created via
SetFunctionReturnType(). This code is actually missing the fact that
at the same time to add dependencies in pg_depend which should add a
link between the function as objid to the type as refobjid. The
reversed dependency is tracked though. Tom, are you working on a
patch? At quick glance, this is just missing a call to
recordDependencyOn() in SetFunctionReturnType().
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 04:53 Tom Lane <tgl@sss.pgh.pa.us>
parent: Michael Paquier <michael.paquier@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2017-06-15 04:53 UTC (permalink / raw)
To: Michael Paquier <michael.paquier@gmail.com>; +Cc: pgsql-bugs
Michael Paquier <michael.paquier@gmail.com> writes:
> On Thu, Jun 15, 2017 at 9:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ... but yeah, as long as we're supporting it at all, that shouldn't
>> happen. Thanks for the report!
> Tom, are you working on a patch? At quick glance, this is just missing a
> call to recordDependencyOn() in SetFunctionReturnType().
No, I'm going to bed soon. If you want to fix this, have at it.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 05:59 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 15+ messages in thread
From: Heikki Linnakangas @ 2017-06-15 05:59 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Karen Huddleston <khuddleston@pivotal.io>; +Cc: pgsql-bugs
On 06/15/2017 04:02 AM, Tom Lane wrote:
> The business with "opaque" as a placeholder has been deprecated for
> circa 15 years; did you find that example in any modern
> documentation?
I wonder if we should remove the support for this, in master. It's been
deprecated for long enough that no-one should miss it when it's gone.
Would be one less hack to maintain.
- Heikki
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 06:05 Michael Paquier <michael.paquier@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2017-06-15 06:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs
On Thu, Jun 15, 2017 at 1:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> On Thu, Jun 15, 2017 at 9:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> ... but yeah, as long as we're supporting it at all, that shouldn't
>>> happen. Thanks for the report!
>
>> Tom, are you working on a patch? At quick glance, this is just missing a
>> call to recordDependencyOn() in SetFunctionReturnType().
>
> No, I'm going to bed soon. If you want to fix this, have at it.
Okay, here you do. The other two call sites of SetFunctionReturnType
involve triggers and PLs, but those are able to consider correctly
dependencies. For PLs:
=# CREATE FUNCTION mylang_call_handler() RETURNS opaque language c AS
'$libdir/plpgsql', $function$plpgsql_call_handler$function$;
CREATE FUNCTION
=# CREATE language mylang HANDLER mylang_call_handler;
WARNING: 42809: changing return type of function mylang_call_handler
from opaque to language_handler
LOCATION: CreateProceduralLanguage, proclang.c:283
CREATE LANGUAGE
=# drop function mylang_call_handler ();
ERROR: 2BP01: cannot drop function mylang_call_handler() because
other objects depend on it
DETAIL: language mylang depends on function mylang_call_handler()
HINT: Use DROP ... CASCADE to drop the dependent objects too.
LOCATION: reportDependentObjects, dependency.c:963
For triggers:
=# CREATE FUNCTION fn_x_before () RETURNS opaque AS $$
BEGIN
return NEW;
END;
$$ LANGUAGE plpgsql;
CREATE FUNCTION
=# CREATE TABLE aa (a int);
CREATE TABLE
=# CREATE TRIGGER aa_before BEFORE INSERT ON aa FOR EACH ROW EXECUTE
PROCEDURE fn_x_before();
WARNING: 01000: changing return type of function fn_x_before from
opaque to trigger
LOCATION: CreateTrigger, trigger.c:551
CREATE TRIGGER
=# DROP FUNCTION fn_x_before ;
ERROR: 2BP01: cannot drop function fn_x_before() because other
objects depend on it
DETAIL: trigger aa_before on table aa depends on function fn_x_before()
HINT: Use DROP ... CASCADE to drop the dependent objects too.
LOCATION: reportDependentObjects, dependency.c:963
Attached is an idea of patch, inputs welcome. Another idea would be to
deprecate things on HEAD and cause an ERROR when doing such things.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Attachments:
[application/octet-stream] type-func-deps.patch (4.6K, ../../CAB7nPqRTpRtypR7gfqYLnmqytC1Xi_=sqnET=YWhSZENrNUz8g@mail.gmail.com/2-type-func-deps.patch)
download | inline diff:
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index e7ecc4ed7e..758a9c1520 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -165,6 +165,8 @@ DefineType(ParseState *pstate, List *names, List *parameters)
Oid resulttype;
ListCell *pl;
ObjectAddress address;
+ bool updated_input = false,
+ updated_output = false;
/*
* As of Postgres 8.4, we require superuser privilege to create a base
@@ -457,6 +459,7 @@ DefineType(ParseState *pstate, List *names, List *parameters)
(errmsg("changing return type of function %s from %s to %s",
NameListToString(inputName), "opaque", typeName)));
SetFunctionReturnType(inputOid, typoid);
+ updated_input = true;
}
else
ereport(ERROR,
@@ -474,6 +477,7 @@ DefineType(ParseState *pstate, List *names, List *parameters)
(errmsg("changing return type of function %s from %s to %s",
NameListToString(outputName), "opaque", "cstring")));
SetFunctionReturnType(outputOid, CSTRINGOID);
+ updated_output = true;
}
else
ereport(ERROR,
@@ -679,6 +683,25 @@ DefineType(ParseState *pstate, List *names, List *parameters)
pfree(array_type);
+ /*
+ * If type of an OPAQUE function has been changed, add cross-dependencies
+ * with the new type created.
+ */
+ if (updated_input)
+ {
+ ObjectAddress input_address;
+
+ ObjectAddressSet(input_address, ProcedureRelationId, inputOid);
+ recordDependencyOn(&input_address, &address, DEPENDENCY_NORMAL);
+ }
+ if (updated_output)
+ {
+ ObjectAddress output_address;
+
+ ObjectAddressSet(output_address, ProcedureRelationId, outputOid);
+ recordDependencyOn(&output_address, &address, DEPENDENCY_NORMAL);
+ }
+
return address;
}
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index 7bdad4e9bb..b2fa8061f3 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -115,6 +115,31 @@ CREATE TYPE not_existing_type (INPUT = array_in,
ELEMENT = int,
INTERNALLENGTH = 32);
ERROR: function array_out(not_existing_type) does not exist
+-- Check dependency transfer of opaque functions when creating a new type
+CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out);
+WARNING: changing argument type of function base_fn_out from "opaque" to base_type
+WARNING: changing return type of function base_fn_in from opaque to base_type
+WARNING: changing return type of function base_fn_out from opaque to cstring
+DROP FUNCTION base_fn_in(cstring); -- error
+ERROR: cannot drop function base_fn_in(cstring) because other objects depend on it
+DETAIL: type base_type depends on function base_fn_in(cstring)
+function base_fn_out(base_type) depends on type base_type
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP FUNCTION base_fn_out(opaque); -- error
+ERROR: function base_fn_out(opaque) does not exist
+DROP TYPE base_type; -- error
+ERROR: cannot drop type base_type because other objects depend on it
+DETAIL: function base_fn_in(cstring) depends on type base_type
+function base_fn_out(base_type) depends on type base_type
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE base_type CASCADE;
+NOTICE: drop cascades to 2 other objects
+DETAIL: drop cascades to function base_fn_in(cstring)
+drop cascades to function base_fn_out(base_type)
-- Check usage of typmod with a user-defined type
-- (we have borrowed numeric's typmod functions)
CREATE TEMP TABLE mytab (foo widget(42,13,7)); -- should fail
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index a1839ef9e7..a28303aa6a 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -115,6 +115,17 @@ CREATE TYPE not_existing_type (INPUT = array_in,
ELEMENT = int,
INTERNALLENGTH = 32);
+-- Check dependency transfer of opaque functions when creating a new type
+CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out);
+DROP FUNCTION base_fn_in(cstring); -- error
+DROP FUNCTION base_fn_out(opaque); -- error
+DROP TYPE base_type; -- error
+DROP TYPE base_type CASCADE;
+
-- Check usage of typmod with a user-defined type
-- (we have borrowed numeric's typmod functions)
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 06:07 Michael Paquier <michael.paquier@gmail.com>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
1 sibling, 0 replies; 15+ messages in thread
From: Michael Paquier @ 2017-06-15 06:07 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Karen Huddleston <khuddleston@pivotal.io>; pgsql-bugs
On Thu, Jun 15, 2017 at 2:59 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 06/15/2017 04:02 AM, Tom Lane wrote:
>>
>> The business with "opaque" as a placeholder has been deprecated for
>> circa 15 years; did you find that example in any modern
>> documentation?
>
>
> I wonder if we should remove the support for this, in master. It's been
> deprecated for long enough that no-one should miss it when it's gone. Would
> be one less hack to maintain.
Yeah, the idea has crossed my mind. And do that as well for PLs and triggers.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 06:51 Michael Paquier <michael.paquier@gmail.com>
parent: Michael Paquier <michael.paquier@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2017-06-15 06:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs
On Thu, Jun 15, 2017 at 3:05 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Attached is an idea of patch, inputs welcome. Another idea would be to
> deprecate things on HEAD and cause an ERROR when doing such things.
Not completely bullet-proof actually. As SetFunctionArgType() can be
called, arguments of a function could be changed, leading to lookup
errors. Attached is an updated patch with more regression tests.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Attachments:
[application/octet-stream] type-func-deps-v2.patch (10.6K, ../../CAB7nPqRUgybqbq-rBcpV_0Pp=YEhAfzxVOc0DoMKp56et4RpNw@mail.gmail.com/2-type-func-deps-v2.patch)
download | inline diff:
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index e7ecc4ed7e..1235ac8019 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -87,8 +87,10 @@ Oid binary_upgrade_next_array_pg_type_oid = InvalidOid;
static void makeRangeConstructors(const char *name, Oid namespace,
Oid rangeOid, Oid subtype);
-static Oid findTypeInputFunction(List *procname, Oid typeOid);
-static Oid findTypeOutputFunction(List *procname, Oid typeOid);
+static Oid findTypeInputFunction(List *procname, Oid typeOid,
+ bool *updatedType);
+static Oid findTypeOutputFunction(List *procname, Oid typeOid,
+ bool *updatedType);
static Oid findTypeReceiveFunction(List *procname, Oid typeOid);
static Oid findTypeSendFunction(List *procname, Oid typeOid);
static Oid findTypeTypmodinFunction(List *procname);
@@ -165,6 +167,8 @@ DefineType(ParseState *pstate, List *names, List *parameters)
Oid resulttype;
ListCell *pl;
ObjectAddress address;
+ bool updated_input = false,
+ updated_output = false;
/*
* As of Postgres 8.4, we require superuser privilege to create a base
@@ -436,8 +440,8 @@ DefineType(ParseState *pstate, List *names, List *parameters)
/*
* Convert I/O proc names to OIDs
*/
- inputOid = findTypeInputFunction(inputName, typoid);
- outputOid = findTypeOutputFunction(outputName, typoid);
+ inputOid = findTypeInputFunction(inputName, typoid, &updated_input);
+ outputOid = findTypeOutputFunction(outputName, typoid, &updated_output);
if (receiveName)
receiveOid = findTypeReceiveFunction(receiveName, typoid);
if (sendName)
@@ -457,6 +461,7 @@ DefineType(ParseState *pstate, List *names, List *parameters)
(errmsg("changing return type of function %s from %s to %s",
NameListToString(inputName), "opaque", typeName)));
SetFunctionReturnType(inputOid, typoid);
+ updated_input = true;
}
else
ereport(ERROR,
@@ -474,6 +479,7 @@ DefineType(ParseState *pstate, List *names, List *parameters)
(errmsg("changing return type of function %s from %s to %s",
NameListToString(outputName), "opaque", "cstring")));
SetFunctionReturnType(outputOid, CSTRINGOID);
+ updated_output = true;
}
else
ereport(ERROR,
@@ -679,6 +685,25 @@ DefineType(ParseState *pstate, List *names, List *parameters)
pfree(array_type);
+ /*
+ * If type of an OPAQUE function has been changed, add cross-dependencies
+ * with the new type created.
+ */
+ if (updated_input)
+ {
+ ObjectAddress input_address;
+
+ ObjectAddressSet(input_address, ProcedureRelationId, inputOid);
+ recordDependencyOn(&input_address, &address, DEPENDENCY_NORMAL);
+ }
+ if (updated_output)
+ {
+ ObjectAddress output_address;
+
+ ObjectAddressSet(output_address, ProcedureRelationId, outputOid);
+ recordDependencyOn(&output_address, &address, DEPENDENCY_NORMAL);
+ }
+
return address;
}
@@ -1631,11 +1656,12 @@ makeRangeConstructors(const char *name, Oid namespace,
* Find suitable I/O functions for a type.
*
* typeOid is the type's OID (which will already exist, if only as a shell
- * type).
+ * type). updatedType tracks if the type of the function found has been
+ * changed.
*/
static Oid
-findTypeInputFunction(List *procname, Oid typeOid)
+findTypeInputFunction(List *procname, Oid typeOid, bool *updatedType)
{
Oid argList[3];
Oid procOid;
@@ -1680,6 +1706,7 @@ findTypeInputFunction(List *procname, Oid typeOid)
(errmsg("changing argument type of function %s from \"opaque\" to \"cstring\"",
NameListToString(procname))));
SetFunctionArgType(procOid, 0, CSTRINGOID);
+ *updatedType = true;
/*
* Need CommandCounterIncrement since DefineType will likely try to
@@ -1702,7 +1729,7 @@ findTypeInputFunction(List *procname, Oid typeOid)
}
static Oid
-findTypeOutputFunction(List *procname, Oid typeOid)
+findTypeOutputFunction(List *procname, Oid typeOid, bool *updatedType)
{
Oid argList[1];
Oid procOid;
@@ -1731,6 +1758,7 @@ findTypeOutputFunction(List *procname, Oid typeOid)
(errmsg("changing argument type of function %s from \"opaque\" to %s",
NameListToString(procname), format_type_be(typeOid))));
SetFunctionArgType(procOid, 0, typeOid);
+ *updatedType = true;
/*
* Need CommandCounterIncrement since DefineType will likely try to
diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out
index 7bdad4e9bb..ed539ec6d7 100644
--- a/src/test/regress/expected/create_type.out
+++ b/src/test/regress/expected/create_type.out
@@ -115,6 +115,76 @@ CREATE TYPE not_existing_type (INPUT = array_in,
ELEMENT = int,
INTERNALLENGTH = 32);
ERROR: function array_out(not_existing_type) does not exist
+-- Check dependency transfer of opaque functions when creating a new type
+CREATE FUNCTION type_arg_opaque_in(opaque) RETURNS opaque AS 'boolin'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION type_arg_opaque_out(opaque) RETURNS cstring AS 'boolout'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION type_return_opaque_in(cstring) RETURNS opaque AS 'boolin'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION type_return_opaque_out(opaque) RETURNS opaque AS 'boolout'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE TYPE type_arg_opaque(INPUT = type_arg_opaque_in,
+ OUTPUT = type_arg_opaque_out);
+WARNING: changing argument type of function type_arg_opaque_in from "opaque" to "cstring"
+WARNING: changing argument type of function type_arg_opaque_out from "opaque" to type_arg_opaque
+WARNING: changing return type of function type_arg_opaque_in from opaque to type_arg_opaque
+CREATE TYPE type_return_opaque(INPUT = type_return_opaque_in,
+ OUTPUT = type_return_opaque_out);
+WARNING: changing argument type of function type_return_opaque_out from "opaque" to type_return_opaque
+WARNING: changing return type of function type_return_opaque_in from opaque to type_return_opaque
+WARNING: changing return type of function type_return_opaque_out from opaque to cstring
+DROP FUNCTION type_arg_opaque_in; -- error
+ERROR: cannot drop function type_arg_opaque_in(cstring) because other objects depend on it
+DETAIL: type type_arg_opaque depends on function type_arg_opaque_in(cstring)
+function type_arg_opaque_out(type_arg_opaque) depends on type type_arg_opaque
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP FUNCTION type_arg_opaque_out; -- error
+ERROR: cannot drop function type_arg_opaque_out(type_arg_opaque) because other objects depend on it
+DETAIL: type type_arg_opaque depends on function type_arg_opaque_out(type_arg_opaque)
+function type_arg_opaque_in(cstring) depends on type type_arg_opaque
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP FUNCTION type_return_opaque_in; -- error
+ERROR: cannot drop function type_return_opaque_in(cstring) because other objects depend on it
+DETAIL: type type_return_opaque depends on function type_return_opaque_in(cstring)
+function type_return_opaque_out(type_return_opaque) depends on type type_return_opaque
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP FUNCTION type_return_opaque_out; -- error
+ERROR: cannot drop function type_return_opaque_out(type_return_opaque) because other objects depend on it
+DETAIL: type type_return_opaque depends on function type_return_opaque_out(type_return_opaque)
+function type_return_opaque_in(cstring) depends on type type_return_opaque
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE type_arg_opaque; -- error
+ERROR: cannot drop type type_arg_opaque because other objects depend on it
+DETAIL: function type_arg_opaque_in(cstring) depends on type type_arg_opaque
+function type_arg_opaque_out(type_arg_opaque) depends on type type_arg_opaque
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE type_return_opaque; -- error
+ERROR: cannot drop type type_return_opaque because other objects depend on it
+DETAIL: function type_return_opaque_in(cstring) depends on type type_return_opaque
+function type_return_opaque_out(type_return_opaque) depends on type type_return_opaque
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE type_arg_opaque CASCADE;
+NOTICE: drop cascades to 2 other objects
+DETAIL: drop cascades to function type_arg_opaque_in(cstring)
+drop cascades to function type_arg_opaque_out(type_arg_opaque)
+DROP TYPE type_return_opaque CASCADE;
+NOTICE: drop cascades to 2 other objects
+DETAIL: drop cascades to function type_return_opaque_in(cstring)
+drop cascades to function type_return_opaque_out(type_return_opaque)
+-- both should report no objects and work
+\df type_arg*
+ List of functions
+ Schema | Name | Result data type | Argument data types | Type
+--------+------+------------------+---------------------+------
+(0 rows)
+
+\df type_return*
+ List of functions
+ Schema | Name | Result data type | Argument data types | Type
+--------+------+------------------+---------------------+------
+(0 rows)
+
-- Check usage of typmod with a user-defined type
-- (we have borrowed numeric's typmod functions)
CREATE TEMP TABLE mytab (foo widget(42,13,7)); -- should fail
diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql
index a1839ef9e7..2c62fffbda 100644
--- a/src/test/regress/sql/create_type.sql
+++ b/src/test/regress/sql/create_type.sql
@@ -115,6 +115,31 @@ CREATE TYPE not_existing_type (INPUT = array_in,
ELEMENT = int,
INTERNALLENGTH = 32);
+-- Check dependency transfer of opaque functions when creating a new type
+CREATE FUNCTION type_arg_opaque_in(opaque) RETURNS opaque AS 'boolin'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION type_arg_opaque_out(opaque) RETURNS cstring AS 'boolout'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION type_return_opaque_in(cstring) RETURNS opaque AS 'boolin'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION type_return_opaque_out(opaque) RETURNS opaque AS 'boolout'
+ LANGUAGE internal IMMUTABLE STRICT;
+CREATE TYPE type_arg_opaque(INPUT = type_arg_opaque_in,
+ OUTPUT = type_arg_opaque_out);
+CREATE TYPE type_return_opaque(INPUT = type_return_opaque_in,
+ OUTPUT = type_return_opaque_out);
+DROP FUNCTION type_arg_opaque_in; -- error
+DROP FUNCTION type_arg_opaque_out; -- error
+DROP FUNCTION type_return_opaque_in; -- error
+DROP FUNCTION type_return_opaque_out; -- error
+DROP TYPE type_arg_opaque; -- error
+DROP TYPE type_return_opaque; -- error
+DROP TYPE type_arg_opaque CASCADE;
+DROP TYPE type_return_opaque CASCADE;
+-- both should report no objects and work
+\df type_arg*
+\df type_return*
+
-- Check usage of typmod with a user-defined type
-- (we have borrowed numeric's typmod functions)
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 07:49 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Michael Paquier <michael.paquier@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Heikki Linnakangas @ 2017-06-15 07:49 UTC (permalink / raw)
To: Michael Paquier <michael.paquier@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs
On 06/15/2017 09:51 AM, Michael Paquier wrote:
> On Thu, Jun 15, 2017 at 3:05 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> Attached is an idea of patch, inputs welcome. Another idea would be to
>> deprecate things on HEAD and cause an ERROR when doing such things.
>
> Not completely bullet-proof actually. As SetFunctionArgType() can be
> called, arguments of a function could be changed, leading to lookup
> errors. Attached is an updated patch with more regression tests.
Hmm. Strictly speaking there's no need to update the dependency when
changing opaque into cstring. Because cstring is a pinned type,
recordDependency will do nothing for it.
But in any case, I think it'd be better and simpler to fix the
dependency in SetFunctionArgType() and SetFunctionReturnType() functions
themselves. They're the ones that change the type, they ought to be
responsible for fixing the dependency too. See attached.
- Heikki
Attachments:
[invalid/octet-stream] 0001-Fix-dependency-when-changing-a-function-s-argument-r.patch (0B, ../../91122adb-3afc-a727-c4c9-478175729f56@iki.fi/2-0001-Fix-dependency-when-changing-a-function-s-argument-r.patch)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 13:20 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
1 sibling, 0 replies; 15+ messages in thread
From: Tom Lane @ 2017-06-15 13:20 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Karen Huddleston <khuddleston@pivotal.io>; pgsql-bugs
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 06/15/2017 04:02 AM, Tom Lane wrote:
>> The business with "opaque" as a placeholder has been deprecated for
>> circa 15 years; did you find that example in any modern
>> documentation?
> I wonder if we should remove the support for this, in master. It's been
> deprecated for long enough that no-one should miss it when it's gone.
> Would be one less hack to maintain.
I thought about that too, but I think it's a bit late for v10.
Wouldn't object to doing it in the v11 cycle. In the meantime,
we'd need a fix for the back branches anyway.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 16:31 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2017-06-15 16:31 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Michael Paquier <michael.paquier@gmail.com>; pgsql-bugs
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> Hmm. Strictly speaking there's no need to update the dependency when
> changing opaque into cstring. Because cstring is a pinned type,
> recordDependency will do nothing for it.
> But in any case, I think it'd be better and simpler to fix the
> dependency in SetFunctionArgType() and SetFunctionReturnType() functions
> themselves. They're the ones that change the type, they ought to be
> responsible for fixing the dependency too. See attached.
Yeah, Heikki's version looks good to me.
Heikki, are you going to commit/backpatch this, or do you want me to?
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-15 16:38 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 15+ messages in thread
From: Heikki Linnakangas @ 2017-06-15 16:38 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Michael Paquier <michael.paquier@gmail.com>; pgsql-bugs
On 06/15/2017 07:31 PM, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> Hmm. Strictly speaking there's no need to update the dependency when
>> changing opaque into cstring. Because cstring is a pinned type,
>> recordDependency will do nothing for it.
>
>> But in any case, I think it'd be better and simpler to fix the
>> dependency in SetFunctionArgType() and SetFunctionReturnType() functions
>> themselves. They're the ones that change the type, they ought to be
>> responsible for fixing the dependency too. See attached.
>
> Yeah, Heikki's version looks good to me.
>
> Heikki, are you going to commit/backpatch this, or do you want me to?
I can do it, thanks.
- Heikki
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-16 01:54 Michael Paquier <michael.paquier@gmail.com>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2017-06-16 01:54 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs
On Fri, Jun 16, 2017 at 1:38 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 06/15/2017 07:31 PM, Tom Lane wrote:
>>
>> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>>>
>>> Hmm. Strictly speaking there's no need to update the dependency when
>>> changing opaque into cstring. Because cstring is a pinned type,
>>> recordDependency will do nothing for it.
>>
>>> But in any case, I think it'd be better and simpler to fix the
>>> dependency in SetFunctionArgType() and SetFunctionReturnType() functions
>>> themselves. They're the ones that change the type, they ought to be
>>> responsible for fixing the dependency too. See attached.
>>
>> Yeah, Heikki's version looks good to me.
>>
>> Heikki, are you going to commit/backpatch this, or do you want me to?
>
> I can do it, thanks.
Fine for me. My first quick guess was better than what I coded at the
end, the idea being to wait after TypeCreate() to get the ObjectAdress
of the new type.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #14706: Dependencies not recorded properly for base types
@ 2017-06-16 08:49 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Michael Paquier <michael.paquier@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Heikki Linnakangas @ 2017-06-16 08:49 UTC (permalink / raw)
To: Michael Paquier <michael.paquier@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs
On 06/16/2017 04:54 AM, Michael Paquier wrote:
> On Fri, Jun 16, 2017 at 1:38 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> On 06/15/2017 07:31 PM, Tom Lane wrote:
>>>
>>> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>>>>
>>>> Hmm. Strictly speaking there's no need to update the dependency when
>>>> changing opaque into cstring. Because cstring is a pinned type,
>>>> recordDependency will do nothing for it.
>>>
>>>> But in any case, I think it'd be better and simpler to fix the
>>>> dependency in SetFunctionArgType() and SetFunctionReturnType() functions
>>>> themselves. They're the ones that change the type, they ought to be
>>>> responsible for fixing the dependency too. See attached.
>>>
>>> Yeah, Heikki's version looks good to me.
>>>
>>> Heikki, are you going to commit/backpatch this, or do you want me to?
>>
>> I can do it, thanks.
>
> Fine for me. My first quick guess was better than what I coded at the
> end, the idea being to wait after TypeCreate() to get the ObjectAdress
> of the new type.
Ok, committed and backported, thanks!
- Heikki
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2017-06-16 08:49 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14 23:22 BUG #14706: Dependencies not recorded properly for base types khuddleston@pivotal.io
2017-06-15 00:42 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-15 01:02 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-15 05:59 ` Heikki Linnakangas <hlinnaka@iki.fi>
2017-06-15 06:07 ` Michael Paquier <michael.paquier@gmail.com>
2017-06-15 13:20 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-15 04:52 ` Michael Paquier <michael.paquier@gmail.com>
2017-06-15 04:53 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-15 06:05 ` Michael Paquier <michael.paquier@gmail.com>
2017-06-15 06:51 ` Michael Paquier <michael.paquier@gmail.com>
2017-06-15 07:49 ` Heikki Linnakangas <hlinnaka@iki.fi>
2017-06-15 16:31 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-15 16:38 ` Heikki Linnakangas <hlinnaka@iki.fi>
2017-06-16 01:54 ` Michael Paquier <michael.paquier@gmail.com>
2017-06-16 08:49 ` Heikki Linnakangas <hlinnaka@iki.fi>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox