Skip site navigation (1) Skip section navigation (2)

Re: reducing NUMERIC size for 9.1

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: reducing NUMERIC size for 9.1
Date: 2010-07-28 19:00:46
Message-ID: 20885.1280343646@sss.pgh.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackers
[ gradually catching up on email ]

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Fri, Jul 16, 2010 at 2:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I don't like the way you did that either (specifically, not the kluge
>> in NUMERIC_DIGITS()). It would probably work better if you declared
>> two different structs, or a union of same, to represent the two layout
>> cases.
>> 
>> n_sign_dscale is now pretty inappropriately named, probably better to
>> change the field name. This will also help to catch anything that's
>> not using the macros. (Renaming the n_weight field, or at least burying
>> it in an extra level of struct, would be helpful for the same reason.)

> I'm not sure what you have in mind here.  If we create a union of two
> structs, we'll still have to pick one of them to use to check the high
> bits of the first word, so I'm not sure we'll be adding all that much
> in terms of clarity.

No, you can do something like this:

typedef struct numeric_short
{
	uint16	word1;
	NumericDigit digits[1];
} numeric_short;

typedef struct numeric_long
{
	uint16	word1;
	int16	weight;
	NumericDigit digits[1];
} numeric_long;

typedef union numeric
{
	uint16	word1;
	numeric_short	short;
	numeric_long	long;
} numeric;

and then access word1 either directly or (after having identified which
format it is) via one of the sub-structs.  If you really wanted to get
pedantic you could have a third sub-struct representing the format for
NaNs, but since those are just going to be word1 it may not be worth the
trouble.

>> It seems like you've handled the NAN case a bit awkwardly. Since the
>> weight is uninteresting for a NAN, it's okay to not store the weight
>> field, so I think what you should do is consider that the dscale field
>> is still full-width, ie the format of the first word remains old-style
>> not new-style. I don't remember whether dscale is meaningful for a NAN,
>> but if it is, your approach is constraining what is possible to store,
>> and is also breaking compatibility with old databases.

> There is only one NaN value.  Neither weight or dscale is meaningful.
> I think if the high two bits of the first word are 11 we never examine
> anything else - do you see somewhere that we're doing otherwise?

I hadn't actually looked.  I think though that it's a mistake to break
compatibility on both dscale and weight when you only need to break one.
Also, weight is *certainly* uninteresting for NaNs since it's not even
meaningful unless there are digits.  dscale could conceivably be worth
something.

>> The sign extension code in the NUMERIC_WEIGHT() macro seems a bit
>> awkward; I wonder if there's a better way. One solution might be to
>> offset the value (ie, add or subtract NUMERIC_SHORT_WEIGHT_MIN) rather
>> than try to sign-extend per se.

> Hmm... so, if the weight is X we store the value
> X-NUMERIC_SHORT_WEIGHT_MIN as an unsigned integer?  That's kind of a
> funny representation - I *think* it works out to sign extension with
> the high bit flipped.  I guess we could do it that way, but it might
> make it harder/more confusing to do bit arithmetic with the weight
> sign bit later on.

Yeah, it was just an idea.  It seems like there should be an easier way
to extract the sign-extended value, though.

			regards, tom lane

In response to

Responses

pgsql-hackers by date

Next:From: Jeff DavisDate: 2010-07-28 19:08:02
Subject: Re: page corruption on 8.3+ that makes it to standby
Previous:From: Robert HaasDate: 2010-07-28 18:50:07
Subject: Re: page corruption on 8.3+ that makes it to standby

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group