Re: Add protransform for numeric, varbit, and temporal types

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Add protransform for numeric, varbit, and temporal types
Date: 2012-02-07 17:43:11
Message-ID: CA+TgmoaPGLz9qHenEE=k7QsrsfZcyjmkeGCWF1Tww+kb2f7i6A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 3, 2012 at 10:31 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Sat, Dec 31, 2011 at 7:36 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>> Building on commit 8f9fe6edce358f7904e0db119416b4d1080a83aa, this adds
>> protransform functions to the length coercions for numeric, varbit, timestamp,
>> timestamptz, time, timetz and interval.  This mostly serves to make more ALTER
>> TABLE ALTER TYPE operations avoid a rewrite, including numeric(10,2) ->
>> numeric(12,2), varbit(4) -> varbit(8) and timestamptz(2) -> timestamptz(4).
>> The rules for varbit are exactly the same as for varchar.  Numeric is slightly
>> more complex:
>>
>>  * Flatten calls to our length coercion function that solely represent
>>  * increases in allowable precision.  Scale changes mutate every datum, so
>>  * they are unoptimizable.  Some values, e.g. 1E-1001, can only fit into an
>>  * unconstrained numeric, so a change from an unconstrained numeric to any
>>  * constrained numeric is also unoptimizable.
>>
>> time{,stamp}{,tz} are similar to varchar for these purposes, except that, for
>> example, plain "timestamptz" is equivalent to "timestamptz(6)".  interval has
>> a vastly different typmod format, but the principles applicable to length
>> coercion remain the same.
>>
>> Under --disable-integer-datetimes, I'm not positive that timestamp_scale() is
>> always a no-op when one would logically expect as much.  Does there exist a
>> timestamp such that v::timestamp(2) differs from v:timestamp(2)::timestamp(4)
>> due to floating point rounding?  Even if so, I'm fairly comfortable calling it
>> a feature rather than a bug to avoid perturbing values that way.
>>
>> After these patches, the only core length coercion casts not having
>> protransform functions are those for "bpchar" and "bit".  For those, we could
>> only optimize trivial cases of no length change.  I'm not planning to do so.
>
> This is cool stuff.  I will plan to review this once the CF starts.

I've committed the numeric and varbit patches and will look at the
temporal one next. I did notice one odd thing, though: sometimes we
seem to get a rebuild on the toast index for no obvious reason:

rhaas=# set client_min_messages=debug1;
SET
rhaas=# create table foo (a serial primary key, b varbit);
NOTICE: CREATE TABLE will create implicit sequence "foo_a_seq" for
serial column "foo.a"
DEBUG: building index "pg_toast_16430_index" on table "pg_toast_16430"
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
"foo_pkey" for table "foo"
DEBUG: building index "foo_pkey" on table "foo"
CREATE TABLE
rhaas=# alter table foo alter column b set data type varbit(4);
DEBUG: rewriting table "foo"
DEBUG: building index "foo_pkey" on table "foo"
ALTER TABLE
rhaas=# alter table foo alter column b set data type varbit;
DEBUG: building index "pg_toast_16430_index" on table "pg_toast_16430"
ALTER TABLE

Strangely, it doesn't happen if I add another column to the table:

rhaas=# set client_min_messages=debug1;
SET
rhaas=# create table foo (a serial primary key, b varbit, c varbit);
NOTICE: CREATE TABLE will create implicit sequence "foo_a_seq" for
serial column "foo.a"
DEBUG: building index "pg_toast_16481_index" on table "pg_toast_16481"
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
"foo_pkey" for table "foo"
DEBUG: building index "foo_pkey" on table "foo"
CREATE TABLE
rhaas=# alter table foo alter column b set data type varbit(4);
DEBUG: building index "pg_toast_16490_index" on table "pg_toast_16490"
DEBUG: rewriting table "foo"
DEBUG: building index "foo_pkey" on table "foo"
ALTER TABLE
rhaas=# alter table foo alter column b set data type varbit;
ALTER TABLE

There may not be any particular harm in a useless rebuild of the TOAST
table index (wasted effort excepted), but my lack of understanding of
why this is happening causes me to fear that there's a bug, not so
much in these patches as in the core alter table code that is enabling
skipping table and index rebuilds.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-02-07 17:47:52 Re: When do we lose column names?
Previous Message Greg Smith 2012-02-07 17:39:19 Re: some longer, larger pgbench tests with various performance-related patches