Re: Arrays of domains

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Arrays of domains
Date: 2017-08-11 17:17:52
Message-ID: 3881.1502471872@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Probably a better answer is to start supporting arrays over domain
> types. That was left unimplemented in the original domains patch,
> but AFAICS not for any better reason than lack of round tuits.

Attached is a patch series that allows us to create arrays of domain
types. I haven't tested this in great detail, so there might be some
additional corners of the system that need work, but it passes basic
sanity checks. I believe it's independent of the other patch I have
in the commitfest for domains over composites, but I haven't tested
for interactions there either.

01-rationalize-coercion-APIs.patch cleans up the APIs of
coerce_to_domain() and some internal functions in parse_coerce.c so that
we consistently pass around a CoercionContext along with CoercionForm.
Previously, we sometimes passed an "isExplicit" boolean flag instead,
which is strictly less information; and coerce_to_domain() didn't even get
that, but instead had to reverse-engineer isExplicit from CoercionForm.
That's contrary to the documentation in primnodes.h that says that
CoercionForm only affects display and not semantics. I don't think this
change fixes any live bugs, but it makes things more consistent. The
main reason for doing it though is that now build_coercion_expression()
receives ccontext, which it needs in order to be able to recursively
invoke coerce_to_target_type(), as required by the next patch.

02-reimplement-ArrayCoerceExpr.patch is the core of the patch. It changes
ArrayCoerceExpr so that the node does not directly know any details of
what has to be done to the individual array elements while performing the
array coercion. Instead, the per-element processing is represented by
a sub-expression whose input is a source array element and whose output
is a target array element. This simplifies life in parse_coerce.c,
because it can build that sub-expression by a recursive invocation of
coerce_to_target_type(), and it allows the executor to handle the
per-element processing as a compiled expression instead of hard-wired
code. This is probably about a wash or a small loss performance-wise
for the simplest case where we just need to invoke one coercion function
for each element. However, there are many cases where the existing code
ends up generating two nested ArrayCoerceExprs, one to do the type
conversion and one to apply a typmod (length) coercion function. In the
new code there will be just one ArrayCoerceExpr, saving one deconstruction
and reconstruction of the array. If I hadn't done it like this, adding
domains into the mix could have produced as many as three
ArrayCoerceExprs, where the top one would have only checked domain
constraints; that did not sound nice performance-wise, and it would have
required a lot of code duplication as well.

Finally, 03-support-arrays-of-domains.patch simply turns on the spigot
by creating an array type in DefineDomain(), and adds some test cases.
Given the new method of handling ArrayCoerceExpr, everything Just Works.

I'll add this to the next commitfest.

regards, tom lane

Attachment Content-Type Size
01-rationalize-coercion-APIs.patch text/x-diff 14.3 KB
02-reimplement-ArrayCoerceExpr.patch text/x-diff 43.4 KB
03-support-arrays-of-domains.patch text/x-diff 9.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-08-11 17:20:43 Re: reload-through-the-top-parent switch the partition table
Previous Message Sokolov Yura 2017-08-11 16:14:22 Re: Lazy hash table for XidInMVCCSnapshot (helps Zipfian a bit)