From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org, Richard Huxton <dev(at)archonet(dot)com> |
Subject: | Re: Domains versus arrays versus typmods |
Date: | 2010-10-20 00:47:19 |
Message-ID: | AANLkTimsV7jU5i0YVD=vhJZiy=aQYKwDaiXxiwo5=DOu@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Oct 19, 2010 at 6:14 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> In bug #5717, Richard Huxton complains that a domain declared like
> CREATE DOMAIN mynums numeric(4,2)[1];
> doesn't work properly, ie, the typmod isn't enforced in places where
> it reasonably ought to be. I dug into this a bit, and found that there
> are more worms in this can than I first expected.
>
> In the first place, plpgsql seems a few bricks shy of a load, in that
> its code to handle assignment to an array element doesn't appear to
> be considering arrays with typmod at all: it just passes typmod -1 to
> exec_simple_cast_value() in the PLPGSQL_DTYPE_ARRAYELEM case in
> exec_assign_value(). Nonetheless, if you try a case like
>
> declare
> x numeric(4,2)[1];
> begin
> x[1] := $1;
>
> you'll find the typmod *is* enforced. It turns out that the reason that
> it works is that after constructing the modified array value,
> exec_assign_value() calls itself recursively to do the actual assignment
> to the array variable --- and that call sees that the array variable has
> a typmod, so it applies the cast *at the array level*, ie it
> disassembles the array, re-coerces each element, and builds a new array.
> So it's horridly inefficient but it works.
>
> That could and should be improved, but it seems to be just a matter
> of local changes in pl_exec.c, and it's only an efficiency hack anyway.
> The big problem is that none of this happens when the variable is
> declared as a domain, and I believe that that is a significantly more
> wide-ranging issue.
>
> The real issue as I see it is that it's possible to subscript an array
> without realizing that the array's type is really a domain that
> incorporates a typmod specification. This happens because of a hack
> we did to simplify implementation of domains over arrays: we expose the
> array element type as typelem of the domain as well. For example, given
> Richard's declaration we have
>
> regression=# select typname,oid,typelem,typbasetype,typtypmod from pg_type where typname = 'mynums';
> typname | oid | typelem | typbasetype | typtypmod
> ---------+-------+---------+-------------+-----------
> mynums | 92960 | 1700 | 1231 | 262150
> (1 row)
>
> If we were to wrap another domain around that, we'd have
>
> regression=# create domain mynums2 as mynums;
> CREATE DOMAIN
> regression=# select typname,oid,typelem,typbasetype,typtypmod from pg_type where typname = 'mynums2';
> typname | oid | typelem | typbasetype | typtypmod
> ---------+-------+---------+-------------+-----------
> mynums2 | 92976 | 1700 | 92960 | -1
> (1 row)
>
> and at this point it's completely unobvious from inspection of the
> pg_type entry that any typmod needs to be applied when subscripting.
>
> So I'm suspicious that there are a boatload of bugs related to this kind
> of domain declaration, not just the one case in plpgsql.
>
> I think that what we ought to do about it is to stop exposing typelem
> in domains' pg_type rows. If you want to subscript a domain value, you
> should have to drill down to its base type with getBaseTypeAndTypmod,
> which would also give you the correct typmod to apply. If we set
> typelem to zero in domain pg_type rows, it shouldn't take too long to
> find any places that are missing this consideration --- the breakage
> will be obvious rather than subtle.
I fear that this is going to degrade the performance of common cases
as a way of debugging rare cases.
> This catalog definitional change obviously shouldn't be back-patched,
> but possibly the ensuing code changes could be, since the typelem change
> is just to expose where things are wrong and wouldn't be a prerequisite
> for corrected code to behave correctly. Or we could decide that this is
> a corner case we're not going to try to fix in back branches. It's been
> wrong since day 0, so there's certainly an argument that it's not
> important enough to risk back-patching.
>
> Comments?
It might be reasonable to back-patch whatever we decide on into 9.0,
because it is so new, but I would be reluctant to go back further
unless we have some evidence that it's bothering people. It seems to
me that this can could have a lot of worms in it, and I fear that
there could be several rounds of fixes, which I would rather not
inflict on users of supposedly-stable branches.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2010-10-20 00:51:16 | Re: WIP: extensible enums |
Previous Message | Hsien-Wen Chu | 2010-10-20 00:39:25 | PostgreSQL and HugePage |