On Thu, Jun 02, 2011 at 06:56:02PM -0400, Tom Lane wrote:
> Noah Misch <n...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
