Thread: Domains versus polymorphic functions, redux
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
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
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
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
"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
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
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
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
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, andsuperficially at least, the above will make it seem like more like than it does with the error. But maybe it's time tore-think how domains are implemented? (Not for 9.1, mind.) I mean, why *don't* they act like first class types? Best, David
"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
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 lifeof its own"? Best, David
"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
lifeof 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
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
Attachment
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 lifeof 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
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
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
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 languageinternal 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
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. Butonce 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
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.
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
On Fri, Jun 3, 2011 at 1:14 AM, Noah Misch <noah@leadboat.com> wrote: > 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. Well, as Bill Clinton once said, "it depends on what the meaning of the word 'is' is". I think of array types in PostgreSQL as meaning "the types whose monikers end in a pair of square brackets". We don't in general have the ability to create a type that behaves "like" another type. In particular, you can't create a user-defined type that "is" an array in the same way that a domain-over-array "is" an array. If we had some kind of type interface facility that might be possible, but we don't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Jun 03, 2011 at 11:22:34AM -0400, Robert Haas wrote: > On Fri, Jun 3, 2011 at 1:14 AM, Noah Misch <noah@leadboat.com> wrote: > > 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. > > Well, as Bill Clinton once said, "it depends on what the meaning of > the word 'is' is". I think of array types in PostgreSQL as meaning > "the types whose monikers end in a pair of square brackets". We don't > in general have the ability to create a type that behaves "like" > another type. In particular, you can't create a user-defined type > that "is" an array in the same way that a domain-over-array "is" an > array. If we had some kind of type interface facility that might be > possible, but we don't. > Early on in this thread, one of the users of domains-over-array-type mentioned that he really didn't want to use them that way, he'd be perfectly happy with array-over-domain: i.e.: mydomain[]. How does that impact all this at the rhetorical level under discussion? Ross -- Ross Reedstrom, Ph.D. reedstrm@rice.edu Systems Engineer & Admin, Research Scientist phone: 713-348-6166 Connexions http://cnx.org fax: 713-348-3665 Rice University MS-375, Houston, TX 77005 GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE
On Jun 3, 2011, at 8:22 AM, Robert Haas wrote: > Well, as Bill Clinton once said, "it depends on what the meaning of > the word 'is' is". I think of array types in PostgreSQL as meaning > "the types whose monikers end in a pair of square brackets". Man, range types are going to fuck with your brainz. D
On Fri, Jun 03, 2011 at 10:42:01AM -0400, Tom Lane wrote: > 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". This already happens today. > 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. I have nothing material to add to my statement in http://archives.postgresql.org/message-id/20110511093217.GB26552@tornado.gateway.2wire.net Let's be concrete; run this on 9.0: CREATE DOMAIN foo AS int[]; SELECT pg_typeof(array_append('{1}'::foo, 1)); SELECT pg_typeof(array_prepend(1, '{1}'::foo)); SELECT pg_typeof(array_fill(1,array[3])); Which of those type resolutions do you find unprincipled, or what hypothetical function does 9.0 resolve in an unprincipled way? (What the function actually does isn't important.) > 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. It would be no good to bifurcate the rules that way. > 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). Perhaps, though it paints us into a backward compatibility corner should we ever support arrays of domains. How many field complaints has the longstanding error outcome produced? > 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. I wrote about the implementation scope in http://archives.postgresql.org/message-id/20110511191249.GA29592@tornado.gateway.2wire.net While I don't doubt the presence of implementation challenges beyond those I identified, we can't meaningfully discuss them as generalities. nm
On Fri, Jun 3, 2011 at 1:00 PM, David E. Wheeler <david@kineticode.com> wrote: > On Jun 3, 2011, at 8:22 AM, Robert Haas wrote: > >> Well, as Bill Clinton once said, "it depends on what the meaning of >> the word 'is' is". I think of array types in PostgreSQL as meaning >> "the types whose monikers end in a pair of square brackets". > > Man, range types are going to fuck with your brainz. They may, but probably not for this reason. Domain types have this weird property that we want all of the base type operations to still work on them, except when we don't want that. Range types won't have that property, or at least I don't think so. Someone might expect 1::foo + 2::foo to work when foo is a domain over int, but they probably won't expect '[1,2]'::intrange + '[2,3)'::intrange to work. The real crux of the issue here is: under what circumstances should we look through the domain wrapper around an underlying type, and under what circumstances should we refrain from doing so? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> The real crux of the issue here is: under what circumstances should we
> look through the domain wrapper around an underlying type, and under
> what circumstances should we refrain from doing so?
That's half of it. The other half is: when we *do* look through the
wrapper, is that equivalent to having implicitly inserted a downcast
to the base type, so that the results are now indistinguishable from
having given a base-type value to begin with? Or is the expression's
behavior still affected by the fact of having given a domain value,
and if so, how exactly?
I assert that matching a domain-over-array to an ANYARRAY parameter
certainly involves having "looked through the wrapper". It's
considerably fuzzier though what should happen when matching a domain
to ANYELEMENT.
regards, tom lane
Robert Haas <robertmhaas@gmail.com> wrote: > The real crux of the issue here is: under what circumstances > should we look through the domain wrapper around an underlying > type, and under what circumstances should we refrain from doing > so? I don't know if this is the intent of domains in the SQL standard, but I find them useful to "extend" other types (in the object-oriented sense). "CaseNoT" *is a* varchar(14), but not every varchar(14) is a "CaseNoT". In my view, behaviors should be determined based on that concept. In the long run, it would be fabulous if a domain could extend another domain -- or event better, multiple other domains with the same base type. DOT hair color code and DOT eye color code domains might both extend a DOT color codes domain. Another long-range nicety would be something which I have seen in some other databases, and which is consistent with the inheritance theme, is that you can't compare or assign dissimilar domains -- an error is thrown. So if you try to join from the eye color column in a person table to the key of a hair color table, you get an error unless you explicitly cast one or both of them to the common type. I know that doesn't directly answer the question, but I think it provides some framework for making choices.... -Kevin
BTW, a possibly relevant point here is that SQL:2008 says (in 4.12)
The purpose of a domain is to constrain the set of valid valuesthat can be stored in a column of a base table by
variousoperations.
and in 4.17.4
A domain constraint is a constraint that is specified for a domain.It is applied to all columns that are based on that
domain,andto all values cast to that domain.
If you take that literally, it means that domain constraints are applied
(1) in an assignment to a table column of a domain type, and
(2) in an explicit CAST to the domain type, and
(3) nowhere else.
In particular I fail to see any support in the spec for the notion that
domain constraints should be applied to the results of expressions that
happen to include a domain value.
In our terms, that definitely suggests that domains should always be
implicitly downcast to base type when passed to polymorphic functions,
so that the result of the function is never automatically of the domain
type.
This all leads to the conclusion that domains are not first-class types
and can't be made so ...
regards, tom lane
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Another long-range nicety would be something which I have seen in
> some other databases, and which is consistent with the inheritance
> theme, is that you can't compare or assign dissimilar domains -- an
> error is thrown. So if you try to join from the eye color column in
> a person table to the key of a hair color table, you get an error
> unless you explicitly cast one or both of them to the common type.
[ raised eyebrow ... ] This is all pretty cute, but I think it goes
against both the letter and spirit of the SQL standard. What you
are describing might be a useful thing to have, but it isn't a SQL
domain.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > A domain constraint is a constraint that is specified for a > domain. > It is applied to all columns that are based on that domain, and > to all values cast to that domain. > > If you take that literally, it means that domain constraints are > applied > (1) in an assignment to a table column of a domain type, and > (2) in an explicit CAST to the domain type, and > (3) nowhere else. I'm curious how you jumped from "all values cast" to "explicit CAST" and "nowhere else". The standard does describe implicit casts. That said, section 4.7.5 talks about supertypes and subtypes, so if I ever want such behavior, it seems to match pretty well (on a quick scan) to what the standard outlines there. No need to invent different mechanisms. -Kevin
On fre, 2011-06-03 at 13:53 -0500, Kevin Grittner wrote: > Another long-range nicety would be something which I have seen in > some other databases, and which is consistent with the inheritance > theme, is that you can't compare or assign dissimilar domains -- an > error is thrown. So if you try to join from the eye color column in > a person table to the key of a hair color table, you get an error > unless you explicitly cast one or both of them to the common type. What you are looking for is the SQL feature called "distinct types". The makers of the SQL standard have sort of deprecated domains in favor of distinct types, because distinct types address your sort of use case better, and prescribing the behavior of domains becomes weirder and weirder as the type system becomes more complex. Which is pretty much the same experience we've been having over the years.
Peter Eisentraut <peter_e@gmx.net> writes:
> What you are looking for is the SQL feature called "distinct types".
> The makers of the SQL standard have sort of deprecated domains in favor
> of distinct types, because distinct types address your sort of use case
> better, and prescribing the behavior of domains becomes weirder and
> weirder as the type system becomes more complex. Which is pretty much
> the same experience we've been having over the years.
Yeah ... the one thing that is actually completely broken about (our
current interpretation of) domains is that a first-class SQL datatype
cannot sanely have a NOT NULL constraint attached to it. That just
doesn't work in conjunction with outer joins, to take one glaring
example.
As I mentioned upthread, a closer look at the standard leads me to think
that the committee doesn't actually intend that a domain's constraints
follow it through operations --- I now think they only intend that the
constraints get checked in the context of a cast to the domain
(including assignment casts). In our terminology that would mean that a
domain gets downcast to its base type as soon as you do anything at all
to the value, even just pass it through a join.
There are certainly applications where such a behavior isn't what you
want, but trying to force domains to do something else is just not going
to lead to desirable results. It's better to invent some other concept,
and it sounds like the committee reached the same conclusion.
Anyway, I think we're out of time to do anything about the issue for
9.1. I think what we'd better do is force a downcast in the context
of matching to an ANYARRAY parameter, and leave the other cases to
revisit later.
regards, tom lane
I wrote:
> Anyway, I think we're out of time to do anything about the issue for
> 9.1. I think what we'd better do is force a downcast in the context
> of matching to an ANYARRAY parameter, and leave the other cases to
> revisit later.
Attached is a draft patch to do the above. It's only lightly tested,
and could use some regression test additions, but it seems to fix
Regina's complaint.
Note that I changed coerce_type's behavior for both ANYARRAY and ANYENUM
targets, but the latter behavioral change is unreachable since the other
routines in parse_coerce.c will not match a domain-over-enum to ANYENUM.
I am half tempted to extend the patch so they will, which would allow
cases like this to work:
regression=# create type color as enum('red','green','blue');
CREATE TYPE
regression=# select enum_first('green'::color);
enum_first
------------
red
(1 row)
regression=# create domain dcolor as color;
CREATE DOMAIN
regression=# select enum_first('green'::dcolor);
ERROR: function enum_first(dcolor) does not exist
LINE 1: select enum_first('green'::dcolor);
^
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
I'm unsure though if there's any support for this further adventure,
since it wouldn't be fixing a 9.1 regression.
Comments?
regards, tom lane
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index 0418972517eee4df52dbdc8f7807aa8fa528a674..e0727f12285d73b3ce48b53ba91cd5d8d9fc87e4 100644
*** a/src/backend/parser/parse_coerce.c
--- b/src/backend/parser/parse_coerce.c
*************** coerce_type(ParseState *pstate, Node *no
*** 143,151 ****
}
if (targetTypeId == ANYOID ||
targetTypeId == ANYELEMENTOID ||
! targetTypeId == ANYNONARRAYOID ||
! (targetTypeId == ANYARRAYOID && inputTypeId != UNKNOWNOID) ||
! (targetTypeId == ANYENUMOID && inputTypeId != UNKNOWNOID))
{
/*
* Assume can_coerce_type verified that implicit coercion is okay.
--- 143,149 ----
}
if (targetTypeId == ANYOID ||
targetTypeId == ANYELEMENTOID ||
! targetTypeId == ANYNONARRAYOID)
{
/*
* Assume can_coerce_type verified that implicit coercion is okay.
*************** coerce_type(ParseState *pstate, Node *no
*** 154,168 ****
* it's OK to treat an UNKNOWN constant as a valid input for a
* function accepting ANY, ANYELEMENT, or ANYNONARRAY. This should be
* all right, since an UNKNOWN value is still a perfectly valid Datum.
- * However an UNKNOWN value is definitely *not* an array, and so we
- * mustn't accept it for ANYARRAY. (Instead, we will call anyarray_in
- * below, which will produce an error.) Likewise, UNKNOWN input is no
- * good for ANYENUM.
*
! * NB: we do NOT want a RelabelType here.
*/
return node;
}
if (inputTypeId == UNKNOWNOID && IsA(node, Const))
{
/*
--- 152,195 ----
* it's OK to treat an UNKNOWN constant as a valid input for a
* function accepting ANY, ANYELEMENT, or ANYNONARRAY. This should be
* all right, since an UNKNOWN value is still a perfectly valid Datum.
*
! * NB: we do NOT want a RelabelType here: the exposed type of the
! * function argument must be its actual type, not the polymorphic
! * pseudotype.
*/
return node;
}
+ if (targetTypeId == ANYARRAYOID ||
+ targetTypeId == ANYENUMOID)
+ {
+ /*
+ * Assume can_coerce_type verified that implicit coercion is okay.
+ *
+ * These cases are unlike the ones above because the exposed type of
+ * the argument must be an actual array or enum type. In particular
+ * the argument must *not* be an UNKNOWN constant. If it is, we just
+ * fall through; below, we'll call anyarray_in or anyenum_in, which
+ * will produce an error. Also, if what we have is a domain over
+ * array or enum, we have to relabel it to its base type.
+ */
+ if (inputTypeId != UNKNOWNOID)
+ {
+ Oid baseTypeId = getBaseType(inputTypeId);
+
+ if (baseTypeId != inputTypeId)
+ {
+ RelabelType *r = makeRelabelType((Expr *) node,
+ baseTypeId, -1,
+ InvalidOid,
+ cformat);
+
+ r->location = location;
+ return (Node *) r;
+ }
+ /* Not a domain type, so return it as-is */
+ return node;
+ }
+ }
if (inputTypeId == UNKNOWNOID && IsA(node, Const))
{
/*
*************** coerce_to_common_type(ParseState *pstate
*** 1257,1262 ****
--- 1284,1294 ----
* (This is a no-op if used in combination with ANYARRAY or ANYENUM, but
* is an extra restriction if not.)
*
+ * Domains over arrays match ANYARRAY, and are immediately flattened to their
+ * base type. (Thus, for example, we will consider it a match if one ANYARRAY
+ * argument is a domain over int4[] while another one is just int4[].) Also
+ * notice that such a domain does *not* match ANYNONARRAY.
+ *
* If we have UNKNOWN input (ie, an untyped literal) for any polymorphic
* argument, assume it is okay.
*
*************** check_generic_type_consistency(Oid *actu
*** 1309,1314 ****
--- 1341,1347 ----
{
if (actual_type == UNKNOWNOID)
continue;
+ actual_type = getBaseType(actual_type); /* flatten domains */
if (OidIsValid(array_typeid) && actual_type != array_typeid)
return false;
array_typeid = actual_type;
*************** check_generic_type_consistency(Oid *actu
*** 1346,1353 ****
if (have_anynonarray)
{
! /* require the element type to not be an array */
! if (type_is_array(elem_typeid))
return false;
}
--- 1379,1386 ----
if (have_anynonarray)
{
! /* require the element type to not be an array or domain over array */
! if (type_is_array_domain(elem_typeid))
return false;
}
*************** check_generic_type_consistency(Oid *actu
*** 1406,1411 ****
--- 1439,1448 ----
* (This is a no-op if used in combination with ANYARRAY or ANYENUM, but
* is an extra restriction if not.)
*
+ * Domains over arrays match ANYARRAY arguments, and are immediately flattened
+ * to their base type. (In particular, if the return type is also ANYARRAY,
+ * we'll set it to the base type not the domain type.)
+ *
* When allow_poly is false, we are not expecting any of the actual_arg_types
* to be polymorphic, and we should not return a polymorphic result type
* either. When allow_poly is true, it is okay to have polymorphic "actual"
*************** enforce_generic_type_consistency(Oid *ac
*** 1485,1490 ****
--- 1522,1528 ----
}
if (allow_poly && decl_type == actual_type)
continue; /* no new information here */
+ actual_type = getBaseType(actual_type); /* flatten domains */
if (OidIsValid(array_typeid) && actual_type != array_typeid)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
*************** enforce_generic_type_consistency(Oid *ac
*** 1557,1564 ****
if (have_anynonarray && elem_typeid != ANYELEMENTOID)
{
! /* require the element type to not be an array */
! if (type_is_array(elem_typeid))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("type matched to anynonarray is an array type: %s",
--- 1595,1602 ----
if (have_anynonarray && elem_typeid != ANYELEMENTOID)
{
! /* require the element type to not be an array or domain over array */
! if (type_is_array_domain(elem_typeid))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("type matched to anynonarray is an array type: %s",
*************** resolve_generic_type(Oid declared_type,
*** 1655,1669 ****
{
if (context_declared_type == ANYARRAYOID)
{
! /* Use actual type, but it must be an array */
! Oid array_typelem = get_element_type(context_actual_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("argument declared \"anyarray\" is not an array but type %s",
! format_type_be(context_actual_type))));
! return context_actual_type;
}
else if (context_declared_type == ANYELEMENTOID ||
context_declared_type == ANYNONARRAYOID ||
--- 1693,1711 ----
{
if (context_declared_type == ANYARRAYOID)
{
! /*
! * Use actual type, but it must be an array; or if it's a domain
! * over array, use the base array type.
! */
! Oid context_base_type = getBaseType(context_actual_type);
! Oid array_typelem = get_element_type(context_base_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("argument declared \"anyarray\" is not an array but type %s",
! format_type_be(context_base_type))));
! return context_base_type;
}
else if (context_declared_type == ANYELEMENTOID ||
context_declared_type == ANYNONARRAYOID ||
*************** resolve_generic_type(Oid declared_type,
*** 1687,1699 ****
if (context_declared_type == ANYARRAYOID)
{
/* Use the element type corresponding to actual type */
! Oid array_typelem = get_element_type(context_actual_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("argument declared \"anyarray\" is not an array but type %s",
! format_type_be(context_actual_type))));
return array_typelem;
}
else if (context_declared_type == ANYELEMENTOID ||
--- 1729,1742 ----
if (context_declared_type == ANYARRAYOID)
{
/* Use the element type corresponding to actual type */
! Oid context_base_type = getBaseType(context_actual_type);
! Oid array_typelem = get_element_type(context_base_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("argument declared \"anyarray\" is not an array but type %s",
! format_type_be(context_base_type))));
return array_typelem;
}
else if (context_declared_type == ANYELEMENTOID ||
*************** IsBinaryCoercible(Oid srctype, Oid targe
*** 1796,1807 ****
/* Also accept any array type as coercible to ANYARRAY */
if (targettype == ANYARRAYOID)
! if (type_is_array(srctype))
return true;
/* Also accept any non-array type as coercible to ANYNONARRAY */
if (targettype == ANYNONARRAYOID)
! if (!type_is_array(srctype))
return true;
/* Also accept any enum type as coercible to ANYENUM */
--- 1839,1850 ----
/* Also accept any array type as coercible to ANYARRAY */
if (targettype == ANYARRAYOID)
! if (type_is_array_domain(srctype))
return true;
/* Also accept any non-array type as coercible to ANYNONARRAY */
if (targettype == ANYNONARRAYOID)
! if (!type_is_array_domain(srctype))
return true;
/* Also accept any enum type as coercible to ANYENUM */
On Tue, Jun 7, 2011 at 6:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>> Anyway, I think we're out of time to do anything about the issue for
>> 9.1. I think what we'd better do is force a downcast in the context
>> of matching to an ANYARRAY parameter, and leave the other cases to
>> revisit later.
>
> Attached is a draft patch to do the above. It's only lightly tested,
> and could use some regression test additions, but it seems to fix
> Regina's complaint.
>
> Note that I changed coerce_type's behavior for both ANYARRAY and ANYENUM
> targets, but the latter behavioral change is unreachable since the other
> routines in parse_coerce.c will not match a domain-over-enum to ANYENUM.
> I am half tempted to extend the patch so they will, which would allow
> cases like this to work:
>
> regression=# create type color as enum('red','green','blue');
> CREATE TYPE
> regression=# select enum_first('green'::color);
> enum_first
> ------------
> red
> (1 row)
>
> regression=# create domain dcolor as color;
> CREATE DOMAIN
> regression=# select enum_first('green'::dcolor);
> ERROR: function enum_first(dcolor) does not exist
> LINE 1: select enum_first('green'::dcolor);
> ^
> HINT: No function matches the given name and argument types. You might need to add explicit type casts.
>
> I'm unsure though if there's any support for this further adventure,
> since it wouldn't be fixing a 9.1 regression.
>
> Comments?
Well, on the one hand, if we're doing it for arrays, it's hard to
imagine that the same behavior for enums can be an outright disaster.
On the flip side, people get reeeeeally cranky about changes that
break application code, so it would not be nice if we had to pull this
one back. How likely is that?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jun 7, 2011 at 6:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Note that I changed coerce_type's behavior for both ANYARRAY and ANYENUM
>> targets, but the latter behavioral change is unreachable since the other
>> routines in parse_coerce.c will not match a domain-over-enum to ANYENUM.
>> I am half tempted to extend the patch so they will, which would allow
>> cases like this to work:
>>
>> regression=# select enum_first('green'::dcolor);
>> ERROR: function enum_first(dcolor) does not exist
> Well, on the one hand, if we're doing it for arrays, it's hard to
> imagine that the same behavior for enums can be an outright disaster.
> On the flip side, people get reeeeeally cranky about changes that
> break application code, so it would not be nice if we had to pull this
> one back. How likely is that?
It's hard to see how allowing this match where there was no match before
would break existing code. A more plausible objection is that we'd be
foreclosing any possibility of handling the match-domain-to-ANYENUM case
differently, since once 9.1 had been out in the field doing this for a
year, you can be sure there *would* be some apps depending on it.
So I think the real question is whether we have totally destroyed the
argument for letting domains pass through polymorphic functions without
getting smashed to their base types. Personally I think that idea is
pretty much dead in the water, but I sense that Noah hasn't given up on
it yet ;-) If we aren't yet willing to treat ANYELEMENT that way, maybe
it's premature to adopt the stance for ANYENUM.
regards, tom lane
On Tue, Jun 7, 2011 at 9:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Tue, Jun 7, 2011 at 6:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Note that I changed coerce_type's behavior for both ANYARRAY and ANYENUM
>>> targets, but the latter behavioral change is unreachable since the other
>>> routines in parse_coerce.c will not match a domain-over-enum to ANYENUM.
>>> I am half tempted to extend the patch so they will, which would allow
>>> cases like this to work:
>>>
>>> regression=# select enum_first('green'::dcolor);
>>> ERROR: function enum_first(dcolor) does not exist
>
>> Well, on the one hand, if we're doing it for arrays, it's hard to
>> imagine that the same behavior for enums can be an outright disaster.
>> On the flip side, people get reeeeeally cranky about changes that
>> break application code, so it would not be nice if we had to pull this
>> one back. How likely is that?
>
> It's hard to see how allowing this match where there was no match before
> would break existing code. A more plausible objection is that we'd be
> foreclosing any possibility of handling the match-domain-to-ANYENUM case
> differently, since once 9.1 had been out in the field doing this for a
> year, you can be sure there *would* be some apps depending on it.
Yes, that's the point I was trying to get at.
> So I think the real question is whether we have totally destroyed the
> argument for letting domains pass through polymorphic functions without
> getting smashed to their base types. Personally I think that idea is
> pretty much dead in the water, but I sense that Noah hasn't given up on
> it yet ;-) If we aren't yet willing to treat ANYELEMENT that way, maybe
> it's premature to adopt the stance for ANYENUM.
Given that we have no field demand for this behavior, maybe it's
better not to add it, so that we have the option later to change our
mind about how it should work.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
I wrote:
>> Anyway, I think we're out of time to do anything about the issue for
>> 9.1. I think what we'd better do is force a downcast in the context
>> of matching to an ANYARRAY parameter, and leave the other cases to
>> revisit later.
> Attached is a draft patch to do the above. It's only lightly tested,
> and could use some regression test additions, but it seems to fix
> Regina's complaint.
Applied with regression-test additions.
regards, tom lane