Domains versus polymorphic functions, redux

Started by Tom Lanealmost 15 years ago37 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

In http://archives.postgresql.org/pgsql-bugs/2011-05/msg00171.php
Regina Obe complains that this fails in 9.1, though it worked before:

regression=# CREATE DOMAIN topoelementarray AS integer[];
CREATE DOMAIN
regression=# SELECT array_upper(ARRAY[[1,2], [3,4]]::topoelementarray, 1);
ERROR: function array_upper(topoelementarray, integer) does not exist

This is a consequence of the changes I made to fix bug #5717,
particularly the issues around ANYARRAY matching discussed here:
http://archives.postgresql.org/pgsql-hackers/2010-10/msg01545.php

Regina is the second or third beta tester to complain about domains over
arrays no longer matching ANYARRAY, so I think we'd better do something
about it. I haven't tried to code anything up yet, but the ideas I'm
considering trying to implement go like this:

1. If a domain type is passed to an ANYARRAY argument, automatically
downcast it to its base type (which of course had better then be an
array). This would include inserting an implicit cast into the
expression tree, so that if the function uses get_fn_expr_argtype or
similar, it would see the base type. Also, if the function returns
ANYARRAY, its result is considered to be of the base type not the
domain.

2. If a domain type is passed to an ANYELEMENT argument, automatically
downcast it to its base type if there is any ANYARRAY argument, or if
the function result type is ANYARRAY, or if any other ANYELEMENT
argument is not of the same domain type. The first two cases are
necessary since we don't have arrays of domains: the match is guaranteed
to fail if we don't do this, since there can be no matching array type
for the domain. The third case is meant to handle cases like
function(domain-over-int, 42) where the function has two ANYELEMENT
arguments: we now fail, but reducing the domain to int would allow
success.

An alternative rule we could use in place of #2 is just "smash domains
to base types always, when they're matched to ANYELEMENT". That would
be simpler and more in keeping with #1, but it might change the behavior
in cases where the historical behavior is reasonable (unlike the cases
discussed in my message referenced above...) I find this simpler rule
tempting from an implementor's standpoint, but am unsure if there'll be
complaints.

Comments, better ideas?

regards, tom lane

#2Merlin Moncure
mmoncure@gmail.com
In reply to: Tom Lane (#1)
Re: Domains versus polymorphic functions, redux

On Tue, May 24, 2011 at 11:12 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

1. If a domain type is passed to an ANYARRAY argument, automatically
downcast it to its base type (which of course had better then be an
array).  This would include inserting an implicit cast into the
expression tree, so that if the function uses get_fn_expr_argtype or
similar, it would see the base type.  Also, if the function returns
ANYARRAY, its result is considered to be of the base type not the
domain.

Does that mean that plpgsql %type variable declarations will see the
base type (and miss any constraint checks?). I think it's fine either
way, but that's worth noting.

An alternative rule we could use in place of #2 is just "smash domains
to base types always, when they're matched to ANYELEMENT".  That would
be simpler and more in keeping with #1, but it might change the behavior
in cases where the historical behavior is reasonable (unlike the cases
discussed in my message referenced above...)  I find this simpler rule
tempting from an implementor's standpoint, but am unsure if there'll be
complaints.

#2a seems cleaner to me (superficially). Got an example of a behavior
you think is changed? In particular, is there a way the new function
would fail where it used to not fail?

merlin

#3David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#1)
Re: Domains versus polymorphic functions, redux

On May 24, 2011, at 9:12 AM, Tom Lane wrote:

An alternative rule we could use in place of #2 is just "smash domains
to base types always, when they're matched to ANYELEMENT". That would
be simpler and more in keeping with #1, but it might change the behavior
in cases where the historical behavior is reasonable (unlike the cases
discussed in my message referenced above...) I find this simpler rule
tempting from an implementor's standpoint, but am unsure if there'll be
complaints.

I'm not sure where the historical behavior manifests, but this certainly seems like it might be the most consistent implementation, as well. Which option is least likely to violate the principal of surprise?

Best,

David

#4Noah Misch
noah@leadboat.com
In reply to: Tom Lane (#1)
Re: Domains versus polymorphic functions, redux

On Tue, May 24, 2011 at 12:12:55PM -0400, Tom Lane wrote:

In http://archives.postgresql.org/pgsql-bugs/2011-05/msg00171.php
Regina Obe complains that this fails in 9.1, though it worked before:

regression=# CREATE DOMAIN topoelementarray AS integer[];
CREATE DOMAIN
regression=# SELECT array_upper(ARRAY[[1,2], [3,4]]::topoelementarray, 1);
ERROR: function array_upper(topoelementarray, integer) does not exist

This is a consequence of the changes I made to fix bug #5717,
particularly the issues around ANYARRAY matching discussed here:
http://archives.postgresql.org/pgsql-hackers/2010-10/msg01545.php

Regina is the second or third beta tester to complain about domains over
arrays no longer matching ANYARRAY, so I think we'd better do something
about it. I haven't tried to code anything up yet, but the ideas I'm
considering trying to implement go like this:

1. If a domain type is passed to an ANYARRAY argument, automatically
downcast it to its base type (which of course had better then be an
array). This would include inserting an implicit cast into the
expression tree, so that if the function uses get_fn_expr_argtype or
similar, it would see the base type. Also, if the function returns
ANYARRAY, its result is considered to be of the base type not the
domain.

We discussed this a few weeks ago:
http://archives.postgresql.org/message-id/20110511093217.GB26552@tornado.gateway.2wire.net

What's to recommend #1 over what I proposed then? Seems like a discard of
functionality for little benefit.

2. If a domain type is passed to an ANYELEMENT argument, automatically
downcast it to its base type if there is any ANYARRAY argument, or if
the function result type is ANYARRAY, or if any other ANYELEMENT
argument is not of the same domain type. The first two cases are
necessary since we don't have arrays of domains: the match is guaranteed
to fail if we don't do this, since there can be no matching array type
for the domain. The third case is meant to handle cases like
function(domain-over-int, 42) where the function has two ANYELEMENT
arguments: we now fail, but reducing the domain to int would allow
success.

This seems generally consistent with other function-resolution rules around
domains. On the other hand, existing users have supposedly coped by adding an
explicit cast to one or the other argument to get the behavior they want. New
applications will quietly get the cast, as it were, on the domain argument(s).
I hesitate to say this is so clearly right as to warrant that change. Even if
it is right, though, this smells like 9.2 material.

nm

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#3)
Re: Domains versus polymorphic functions, redux

"David E. Wheeler" <david@kineticode.com> writes:

On May 24, 2011, at 9:12 AM, Tom Lane wrote:

An alternative rule we could use in place of #2 is just "smash domains
to base types always, when they're matched to ANYELEMENT". That would
be simpler and more in keeping with #1, but it might change the behavior
in cases where the historical behavior is reasonable (unlike the cases
discussed in my message referenced above...) I find this simpler rule
tempting from an implementor's standpoint, but am unsure if there'll be
complaints.

I'm not sure where the historical behavior manifests, but this
certainly seems like it might be the most consistent implementation,
as well. Which option is least likely to violate the principal of
surprise?

Well, the basic issue here is what happens when a function like

create function noop(anyelement) returns anyelement ...

is applied to a domain argument. Currently, the result is thought to be
of the domain type, whereas if we smash to base unconditionally, the
result will be thought to be of the domain's base type. You can make an
argument for either behavior, but I think the argument for the current
behavior hinges on the assumption that such a function isn't "doing
anything" to the argument value, only passing it through as-is.

I should probably also point out the previous discussion of this area
from a couple weeks ago, notably here:
http://archives.postgresql.org/pgsql-hackers/2011-05/msg00640.php
The example I gave there seems relevant:

create function negate(anyelement) returns anyelement as
$$ select - $1 $$ language sql;

create domain pos as int check (value > 0);

select negate(42::pos);

This example function isn't quite silly --- it will work on any datatype
having a unary '-' operator, and you could imagine someone wanting to do
something roughly like this in more realistic cases. But if you want to
assume that the function returns pos when handed pos, you'd better be
prepared to insert a CastToDomain node to recheck the domain constraint.
Right now the SQL-function code doesn't support such cases:

regression=# select negate(42::pos);
ERROR: return type mismatch in function declared to return pos
DETAIL: Actual return type is integer.
CONTEXT: SQL function "negate" during inlining

If we smashed to base type then this issue would go away.

On the other hand it feels like we'd be taking yet another step away
from allowing domains to be usefully used in function declarations.
I can't put my finger on any concrete consequence of that sort, since
what we're talking about here is ANYELEMENT/ANYARRAY functions not
functions declared to take domains --- but it sure seems like this
would put domains even further away from the status of first-class
citizenship in the type system.

regards, tom lane

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Merlin Moncure (#2)
Re: Domains versus polymorphic functions, redux

Merlin Moncure <mmoncure@gmail.com> writes:

On Tue, May 24, 2011 at 11:12 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

1. If a domain type is passed to an ANYARRAY argument, automatically
downcast it to its base type (which of course had better then be an
array).

Does that mean that plpgsql %type variable declarations will see the
base type (and miss any constraint checks?).

No, this has nothing to do with %type. What's at stake is matching to
functions/operators that are declared to take ANYARRAY.

#2a seems cleaner to me (superficially). Got an example of a behavior
you think is changed?

See my response to David Wheeler.

regards, tom lane

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noah Misch (#4)
Re: Domains versus polymorphic functions, redux

Noah Misch <noah@leadboat.com> writes:

On Tue, May 24, 2011 at 12:12:55PM -0400, Tom Lane wrote:

This is a consequence of the changes I made to fix bug #5717,
particularly the issues around ANYARRAY matching discussed here:
http://archives.postgresql.org/pgsql-hackers/2010-10/msg01545.php

We discussed this a few weeks ago:
http://archives.postgresql.org/message-id/20110511093217.GB26552@tornado.gateway.2wire.net

What's to recommend #1 over what I proposed then? Seems like a discard of
functionality for little benefit.

I am unwilling to commit to making #2 work, especially not under time
constraints; and you apparently aren't either, since you haven't
produced the patch you alluded to at the end of that thread. Even if
you had, though, I'd have no confidence that all holes of the sort had
been closed. What you're proposing is to ratchet up the implementation
requirements for every PL and and every C function declared to accept
polymorphic types, and there are a lot of members of both classes that
we don't control.

I hesitate to say this is so clearly right as to warrant that change. Even if
it is right, though, this smells like 9.2 material.

Well, I'd been hoping to leave it for later too, but it seems like we
have to do something about the ANYARRAY case for 9.1. Making ANYARRAY's
response to domains significantly inconsistent with ANYELEMENT's
response doesn't seem like a good plan.

regards, tom lane

#8Noah Misch
noah@leadboat.com
In reply to: Tom Lane (#7)
Re: Domains versus polymorphic functions, redux

On Tue, May 24, 2011 at 01:28:38PM -0400, Tom Lane wrote:

Noah Misch <noah@leadboat.com> writes:

On Tue, May 24, 2011 at 12:12:55PM -0400, Tom Lane wrote:

This is a consequence of the changes I made to fix bug #5717,
particularly the issues around ANYARRAY matching discussed here:
http://archives.postgresql.org/pgsql-hackers/2010-10/msg01545.php

We discussed this a few weeks ago:
http://archives.postgresql.org/message-id/20110511093217.GB26552@tornado.gateway.2wire.net

What's to recommend #1 over what I proposed then? Seems like a discard of
functionality for little benefit.

I am unwilling to commit to making #2 work, especially not under time
constraints; and you apparently aren't either, since you haven't
produced the patch you alluded to at the end of that thread.

I took your lack of any response as non-acceptance of the plan I outlined.
Alas, the wrong conclusion. I'll send a patch this week.

Even if
you had, though, I'd have no confidence that all holes of the sort had
been closed. What you're proposing is to ratchet up the implementation
requirements for every PL and and every C function declared to accept
polymorphic types, and there are a lot of members of both classes that
we don't control.

True. I will not give you that confidence. Those omissions would have to
remain bugs to be fixed as they're found.

nm

#9David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#5)
Re: Domains versus polymorphic functions, redux

On May 24, 2011, at 10:11 AM, Tom Lane wrote:

regression=# select negate(42::pos);
ERROR: return type mismatch in function declared to return pos
DETAIL: Actual return type is integer.
CONTEXT: SQL function "negate" during inlining

If we smashed to base type then this issue would go away.

+1

On the other hand it feels like we'd be taking yet another step away
from allowing domains to be usefully used in function declarations.
I can't put my finger on any concrete consequence of that sort, since
what we're talking about here is ANYELEMENT/ANYARRAY functions not
functions declared to take domains --- but it sure seems like this
would put domains even further away from the status of first-class
citizenship in the type system.

I agree. It sure seems to me like DOMAINs should act exactly like any other type. I know that has improved over time, and superficially at least, the above will make it seem like more like than it does with the error. But maybe it's time to re-think how domains are implemented? (Not for 9.1, mind.) I mean, why *don't* they act like first class types?

Best,

David

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#9)
Re: Domains versus polymorphic functions, redux

"David E. Wheeler" <david@kineticode.com> writes:

On May 24, 2011, at 10:11 AM, Tom Lane wrote:

On the other hand it feels like we'd be taking yet another step away
from allowing domains to be usefully used in function declarations.

I agree. It sure seems to me like DOMAINs should act exactly like any
other type. I know that has improved over time, and superficially at
least, the above will make it seem like more like than it does with
the error. But maybe it's time to re-think how domains are
implemented? (Not for 9.1, mind.) I mean, why *don't* they act like
first class types?

Well, if they actually were first-class types, they probably wouldn't
be born with an implicit cast to some other type to handle 99% of
operations on them ;-). I think the hard part here is having that cake
and eating it too, ie, supporting domain-specific functions without
breaking the implicit use of the base type's functions.

I guess that the question that's immediately at hand is sort of a
variant of that, because using a polymorphic function declared to take
ANYARRAY on a domain-over-array really is using a portion of the base
type's functionality. What we've learned from bug #5717 and the
subsequent issues is that using that base functionality without
immediately abandoning the notion that the domain has some life of its
own (ie, immediately casting to the base type) is harder than it looks.

regards, tom lane

#11David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#10)
Re: Domains versus polymorphic functions, redux

On May 24, 2011, at 11:30 AM, Tom Lane wrote:

Well, if they actually were first-class types, they probably wouldn't
be born with an implicit cast to some other type to handle 99% of
operations on them ;-). I think the hard part here is having that cake
and eating it too, ie, supporting domain-specific functions without
breaking the implicit use of the base type's functions.

Yeah.

I guess that the question that's immediately at hand is sort of a
variant of that, because using a polymorphic function declared to take
ANYARRAY on a domain-over-array really is using a portion of the base
type's functionality. What we've learned from bug #5717 and the
subsequent issues is that using that base functionality without
immediately abandoning the notion that the domain has some life of its
own (ie, immediately casting to the base type) is harder than it looks.

Well, in the ANYELEMENT context (or ANYARRAY), what could be lost by "abandoning the notion that the domain has some life of its own"?

Best,

David

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#11)
Re: Domains versus polymorphic functions, redux

"David E. Wheeler" <david@kineticode.com> writes:

On May 24, 2011, at 11:30 AM, Tom Lane wrote:

I guess that the question that's immediately at hand is sort of a
variant of that, because using a polymorphic function declared to take
ANYARRAY on a domain-over-array really is using a portion of the base
type's functionality. What we've learned from bug #5717 and the
subsequent issues is that using that base functionality without
immediately abandoning the notion that the domain has some life of its
own (ie, immediately casting to the base type) is harder than it looks.

Well, in the ANYELEMENT context (or ANYARRAY), what could be lost by "abandoning the notion that the domain has some life of its own"?

I'm starting to think that maybe we should separate the two cases after
all. If we force a downcast for ANYARRAY matching, we will fix the loss
of functionality induced by the bug #5717 patch, and it doesn't seem
like anyone has a serious objection to that. What to do for ANYELEMENT
seems to be a bit more controversial, and at least some of the proposals
aren't reasonable to do in 9.1 at this stage. Maybe we should just
leave ANYELEMENT as-is for the moment, and reconsider that issue later?

regards, tom lane

#13Noah Misch
noah@leadboat.com
In reply to: Noah Misch (#8)
Re: Domains versus polymorphic functions, redux

On Tue, May 24, 2011 at 02:00:54PM -0400, Noah Misch wrote:

On Tue, May 24, 2011 at 01:28:38PM -0400, Tom Lane wrote:

Noah Misch <noah@leadboat.com> writes:

On Tue, May 24, 2011 at 12:12:55PM -0400, Tom Lane wrote:

This is a consequence of the changes I made to fix bug #5717,
particularly the issues around ANYARRAY matching discussed here:
http://archives.postgresql.org/pgsql-hackers/2010-10/msg01545.php

We discussed this a few weeks ago:
http://archives.postgresql.org/message-id/20110511093217.GB26552@tornado.gateway.2wire.net

What's to recommend #1 over what I proposed then? Seems like a discard of
functionality for little benefit.

I am unwilling to commit to making #2 work, especially not under time
constraints; and you apparently aren't either, since you haven't
produced the patch you alluded to at the end of that thread.

I took your lack of any response as non-acceptance of the plan I outlined.
Alas, the wrong conclusion. I'll send a patch this week.

See attached arrdom1v1-polymorphism.patch. This currently adds one syscache
lookup per array_append, array_prepend or array_cat call when the anyarray
type is not a domain. When the type is a domain, it adds a few more. We
could add caching without too much trouble. I suppose someone out there uses
these functions in bulk operations, though I've yet to see it. Is it worth
optimizing this straightway?

For a function like
CREATE FUNCTION f(anyarray, VARIADIC anyarray) RETURNS int LANGUAGE sql
AS 'SELECT array_length($1, 1) + array_length($2, 1)'
we must coerce the variadic argument array to a domain type when the other
anyarray argument(s) compel it. Having implemented that, it was nearly free
to re-support a VARIADIC parameter specifically declared with a domain over an
array. Consequently, I've done that as well.

See here for previously-disclosed rationale:
http://archives.postgresql.org/message-id/20110511191249.GA29592@tornado.gateway.2wire.net

I audited remaining get_element_type() callers. CheckAttributeType() needs to
recurse into domains over array types just like any other array type. Fixed
trivially in arrdom2v1-checkattr.patch; see its test case for an example hole.

nm

Attachments:

arrdom1v1-polymorphism.patchtext/plain; charset=us-asciiDownload+139-51
arrdom2v1-checkattr.patchtext/plain; charset=us-asciiDownload+7-2
#14Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#12)
Re: Domains versus polymorphic functions, redux

On Tue, May 24, 2011 at 2:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David E. Wheeler" <david@kineticode.com> writes:

On May 24, 2011, at 11:30 AM, Tom Lane wrote:

I guess that the question that's immediately at hand is sort of a
variant of that, because using a polymorphic function declared to take
ANYARRAY on a domain-over-array really is using a portion of the base
type's functionality.  What we've learned from bug #5717 and the
subsequent issues is that using that base functionality without
immediately abandoning the notion that the domain has some life of its
own (ie, immediately casting to the base type) is harder than it looks.

Well, in the ANYELEMENT context (or ANYARRAY), what could be lost by "abandoning the notion that the domain has some life of its own"?

I'm starting to think that maybe we should separate the two cases after
all.  If we force a downcast for ANYARRAY matching, we will fix the loss
of functionality induced by the bug #5717 patch, and it doesn't seem
like anyone has a serious objection to that.  What to do for ANYELEMENT
seems to be a bit more controversial, and at least some of the proposals
aren't reasonable to do in 9.1 at this stage.  Maybe we should just
leave ANYELEMENT as-is for the moment, and reconsider that issue later?

If we haven't lost any functionality with respect to ANYELEMENT in
9.1, then I don't think we ought to try to improve/change/break it in
9.1 either. But I do think we need to do something about ANYARRAY
matching, and your proposed fix seems pretty reasonable to me.

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

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#14)
Re: Domains versus polymorphic functions, redux

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, May 24, 2011 at 2:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm starting to think that maybe we should separate the two cases after
all. �If we force a downcast for ANYARRAY matching, we will fix the loss
of functionality induced by the bug #5717 patch, and it doesn't seem
like anyone has a serious objection to that. �What to do for ANYELEMENT
seems to be a bit more controversial, and at least some of the proposals
aren't reasonable to do in 9.1 at this stage. �Maybe we should just
leave ANYELEMENT as-is for the moment, and reconsider that issue later?

If we haven't lost any functionality with respect to ANYELEMENT in
9.1, then I don't think we ought to try to improve/change/break it in
9.1 either. But I do think we need to do something about ANYARRAY
matching, and your proposed fix seems pretty reasonable to me.

Yeah, the thread seems to have died off without anyone having a better
idea. I'll see about making this happen.

regards, tom lane

#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noah Misch (#13)
Re: Domains versus polymorphic functions, redux

Noah Misch <noah@leadboat.com> writes:

On Tue, May 24, 2011 at 02:00:54PM -0400, Noah Misch wrote:

I took your lack of any response as non-acceptance of the plan I outlined.
Alas, the wrong conclusion. I'll send a patch this week.

See attached arrdom1v1-polymorphism.patch. This currently adds one syscache
lookup per array_append, array_prepend or array_cat call when the anyarray
type is not a domain. When the type is a domain, it adds a few more. We
could add caching without too much trouble. I suppose someone out there uses
these functions in bulk operations, though I've yet to see it. Is it worth
optimizing this straightway?

I took another look at this, and I think I fundamentally don't agree
with the approach you're taking, quite aside from any implementation
difficulties or performance penalties. It makes more sense to me for
operations like array_cat to downcast their arguments to plain arrays.
If you then assign the result to a target of the domain-over-array type,
there will be an automatic upcast to the domain type, and then any
constraints on the domain will be rechecked at that time. If you don't,
well, it's an array value. The direction you want to go here makes as
little sense as arguing that

create domain pos as int check (value > 0);

select 2::pos - 42;

ought to fail because somehow the domain should override the result type
of the subtraction operator.

So I'm back to thinking that alternative #1 (downcast a domain to its
base array type when matching to an ANYARRAY argument) is the way to go.

I audited remaining get_element_type() callers. CheckAttributeType() needs to
recurse into domains over array types just like any other array type. Fixed
trivially in arrdom2v1-checkattr.patch; see its test case for an example hole.

Yeah, that is a bug; I applied a slightly different patch for it.

I'm not convinced whether any of the get_element_type ->
get_base_element_type changes in your first patch are needed. When
I made the don't-expose-the-typelem change originally, I intentionally
created a separate function because I thought that most existing callers
shouldn't look through domain types. I'm not convinced that a
wholesale readjustment of those callers is appropriate.

regards, tom lane

#17Noah Misch
noah@leadboat.com
In reply to: Tom Lane (#16)
Re: Domains versus polymorphic functions, redux

On Thu, Jun 02, 2011 at 06:56:02PM -0400, Tom Lane wrote:

Noah Misch <noah@leadboat.com> writes:

On Tue, May 24, 2011 at 02:00:54PM -0400, Noah Misch wrote:

I took your lack of any response as non-acceptance of the plan I outlined.
Alas, the wrong conclusion. I'll send a patch this week.

See attached arrdom1v1-polymorphism.patch. This currently adds one syscache
lookup per array_append, array_prepend or array_cat call when the anyarray
type is not a domain. When the type is a domain, it adds a few more. We
could add caching without too much trouble. I suppose someone out there uses
these functions in bulk operations, though I've yet to see it. Is it worth
optimizing this straightway?

I took another look at this, and I think I fundamentally don't agree
with the approach you're taking, quite aside from any implementation
difficulties or performance penalties. It makes more sense to me for
operations like array_cat to downcast their arguments to plain arrays.
If you then assign the result to a target of the domain-over-array type,
there will be an automatic upcast to the domain type, and then any
constraints on the domain will be rechecked at that time. If you don't,
well, it's an array value.

I don't doubt that's usable, and folks would find the behavior of their
applications acceptable given that approach. However, it's an arbitrary (from
the user's perspective) difference in behavior compared to the interaction of
polymorphic functions with domains over scalars. You did propose removing that
inconsistency, but that just builds up a fresh inconsistency between domains
over scalars and plain scalars. And for what gain apart from implementation
ease or performance improvements?

The direction you want to go here makes as
little sense as arguing that

create domain pos as int check (value > 0);

select 2::pos - 42;

ought to fail because somehow the domain should override the result type
of the subtraction operator.

I'm only contending for the behavior of polymorphic functions. We'll always
downcast a domain over int to use an (int, int) function, and we'll always
downcast a domain over int[] to use an (int[], int[]) function. The question is
whether we also downcast before using an (anyarray, anyarray) function. If you
update your example like this, it mirrors my argument:

create domain pos as int check (value > 0);
create function subany(anyelement, anyelement) returns anyelement
language internal as 'implementation_left_as_exercise';

select subany(2::pos, 42::pos);

That had better fail. Currently, whether it does so is up to the C code
implementing the function, just like I propose be the case for arrays.

So I'm back to thinking that alternative #1 (downcast a domain to its
base array type when matching to an ANYARRAY argument) is the way to go.

I audited remaining get_element_type() callers. CheckAttributeType() needs to
recurse into domains over array types just like any other array type. Fixed
trivially in arrdom2v1-checkattr.patch; see its test case for an example hole.

Yeah, that is a bug; I applied a slightly different patch for it.

Looks better; thanks.

I'm not convinced whether any of the get_element_type ->
get_base_element_type changes in your first patch are needed. When
I made the don't-expose-the-typelem change originally, I intentionally
created a separate function because I thought that most existing callers
shouldn't look through domain types. I'm not convinced that a
wholesale readjustment of those callers is appropriate.

How would you restructure those checks, supposing you were writing a similar
patch to allow domains for the cases under examination at those call sites?
Every site I changed needed to comprehend domains over arrays, though doubtless
there are other ways to make them do so.

Thanks,
nm

#18Robert Haas
robertmhaas@gmail.com
In reply to: Noah Misch (#17)
Re: Domains versus polymorphic functions, redux

On Thu, Jun 2, 2011 at 8:25 PM, Noah Misch <noah@leadboat.com> wrote:

I don't doubt that's usable, and folks would find the behavior of their
applications acceptable given that approach.  However, it's an arbitrary (from
the user's perspective) difference in behavior compared to the interaction of
polymorphic functions with domains over scalars.  You did propose removing that
inconsistency, but that just builds up a fresh inconsistency between domains
over scalars and plain scalars.  And for what gain apart from implementation
ease or performance improvements?

Perhaps this is stating the obvious, but implementation ease and
performance improvements can sometimes be non-trivial benefits.

But in this case I really don't quite understand why you don't like
the proposed behavior. AIUI, the case we're talking about is a
function foo that takes, say, anyarray, and returns anyarray.

Now, let us say we attempt to call foo(bar), bar being a type name.
What happens? Well, in general, you get back an error saying that no
such function exists, because bar is not an array type. Certainly if
bar is int4 or box or a composite type or a domain over a scalar type,
we are done for. The function call does not match and nothing is left
to us to throw an error. The only way that this call can possibly
match is if bar happens to be a domain over an array type, and we
treat the call as a request to smash bar to the underlying array type.
And it seems that is the behavior people want. But once we've done
that, I don't see why we should then feel compelled to also insert a
cast in the other direction. In fact, doing so would seem totally
arbitrary and counterintuitive.

I mean, suppose bar is a domain over int[]. If the user had called
foo(some_bar_value::int[]), what should the return type be?
Presumably int[], no? If he called
foo(some_bar_value::int[]::numeric[]), then the return value should be
numeric[], I would think. So if he just calls foo(some_bar_value),
what should the return type be? Well, again, to make the function
call match at all, we have to insert a cast there... otherwise the
input is not of an array type and the call should just fail. And once
we've inserted the cast, then ISTM that we're bound to make the return
type match the cast we stuck in there.

I might be all wet here... but that's how it seems to me ATM.

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

#19Noah Misch
noah@leadboat.com
In reply to: Robert Haas (#18)
Re: Domains versus polymorphic functions, redux

On Fri, Jun 03, 2011 at 12:27:35AM -0400, Robert Haas wrote:

But in this case I really don't quite understand why you don't like
the proposed behavior. AIUI, the case we're talking about is a
function foo that takes, say, anyarray, and returns anyarray.

Now, let us say we attempt to call foo(bar), bar being a type name.
What happens? Well, in general, you get back an error saying that no
such function exists, because bar is not an array type. Certainly if
bar is int4 or box or a composite type or a domain over a scalar type,
we are done for. The function call does not match and nothing is left
to us to throw an error. The only way that this call can possibly
match is if bar happens to be a domain over an array type,

Correct so far.

and we
treat the call as a request to smash bar to the underlying array type.

No, there's no need to do that. The domain "is" an array, not merely something
that can be coerced to an array. Therefore, it can be chosen as the polymorphic
type directly. Indeed, all released versions do this.

And it seems that is the behavior people want. But once we've done
that, I don't see why we should then feel compelled to also insert a
cast in the other direction. In fact, doing so would seem totally
arbitrary and counterintuitive.

I agree. Both the argument and the return type need to be resolved the same
way, either both as the domain or both as the base array type.

#20Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noah Misch (#19)
Re: Domains versus polymorphic functions, redux

Noah Misch <noah@leadboat.com> writes:

On Fri, Jun 03, 2011 at 12:27:35AM -0400, Robert Haas wrote:

and we
treat the call as a request to smash bar to the underlying array type.

No, there's no need to do that. The domain "is" an array, not merely
something that can be coerced to an array. Therefore, it can be
chosen as the polymorphic type directly. Indeed, all released
versions do this.

No, it cannot "be chosen as the polymorphic type directly". The problem
with that is that there is no principled way to resolve ANYELEMENT
unless you consider that you have downcasted the domain to the array
type. You could perhaps ignore that problem for polymorphic functions
that use only ANYARRAY and not ANYELEMENT in their arguments and return
type --- but I'm not happy with the idea that that case would work
differently from a function that does use both.

So far as the other points you raise are concerned, I'm still of the
opinion that we might be best off to consider that domains should be
smashed to their base types when matching them to ANYELEMENT, too.
Again, if we don't do that, we have a problem with figuring out what
ANYARRAY ought to be (since we don't create an array type to go with a
domain). More generally, this dodges the whole problem of needing
polymorphic functions to enforce domain constraints, something I still
believe is entirely impractical from an implementation standpoint.

regards, tom lane

#21Robert Haas
robertmhaas@gmail.com
In reply to: Noah Misch (#19)
#22Ross J. Reedstrom
reedstrm@rice.edu
In reply to: Robert Haas (#21)
#23David E. Wheeler
david@kineticode.com
In reply to: Robert Haas (#21)
#24Noah Misch
noah@leadboat.com
In reply to: Tom Lane (#20)
#25Robert Haas
robertmhaas@gmail.com
In reply to: David E. Wheeler (#23)
#26Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#25)
#27Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Robert Haas (#25)
#28Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#26)
#29Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kevin Grittner (#27)
#30Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Tom Lane (#28)
#31Peter Eisentraut
peter_e@gmx.net
In reply to: Kevin Grittner (#27)
#32Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#31)
#33Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#32)
#34Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#33)
#35Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#34)
#36Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#35)
#37Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#33)