Re: WIP: Relaxing the constraints on numeric scale
On Fri, 23 Jul 2021 at 16:50, Tom Lane wrote: > > OK, I've now studied this more closely, and have some additional > nitpicks: > > * I felt the way you did the documentation was confusing. It seems > better to explain the normal case first, and then describe the two > extended cases. OK, that looks much better. Re-reading the entire section, I think it's much clearer now. > * As long as we're encapsulating typmod construction/extraction, let's > also encapsulate the checks for valid typmods. Good idea. > * Other places are fairly careful to declare typmod values as "int32", > so I think this code should too. OK, that seems sensible. > Attached is a proposed delta patch making those changes. > > (I made the docs mention that the extension cases are allowed as of v15. > While useful in the short run, that will look like noise in ten years; > so I could go either way on whether to do that.) Hmm, yeah. In general,I find such things in the documentation useful for quite a few years. I'm regularly looking to see when a particular feature was added, to see if I can use it in a particular situation. But eventually, it'll become irrelevant, and I don't know if anyone will go around tidying these things up. I have left it in, but perhaps there is a wider discussion to be had about whether we should be doing that more (or less) often. FWIW, I like the way some docs include an "available since" tag (e.g,, Java's @since tag). > If you're good with these, then I think it's ready to go. > I'll mark it RfC in the commitfest. Thanks. That all looked good, so I have pushed it. Regards, Dean
Re: WIP: Relaxing the constraints on numeric scale
Dean Rasheed writes: > All your other suggestions make sense too. Attached is a new version. OK, I've now studied this more closely, and have some additional nitpicks: * I felt the way you did the documentation was confusing. It seems better to explain the normal case first, and then describe the two extended cases. * As long as we're encapsulating typmod construction/extraction, let's also encapsulate the checks for valid typmods. * Other places are fairly careful to declare typmod values as "int32", so I think this code should too. Attached is a proposed delta patch making those changes. (I made the docs mention that the extension cases are allowed as of v15. While useful in the short run, that will look like noise in ten years; so I could go either way on whether to do that.) If you're good with these, then I think it's ready to go. I'll mark it RfC in the commitfest. regards, tom lane diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index 6abda2f1d2..d3c70667a3 100644 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -545,8 +545,8 @@ NUMERIC(precision, scale) - The precision must be positive, the scale may be positive or negative - (see below). Alternatively: + The precision must be positive, while the scale may be positive or + negative (see below). Alternatively: NUMERIC(precision) @@ -569,8 +569,8 @@ NUMERIC The maximum precision that can be explicitly specified in - a NUMERIC type declaration is 1000. An - unconstrained NUMERIC column is subject to the limits + a numeric type declaration is 1000. An + unconstrained numeric column is subject to the limits described in . @@ -578,38 +578,48 @@ NUMERIC If the scale of a value to be stored is greater than the declared scale of the column, the system will round the value to the specified - number of fractional digits. If the declared scale of the column is - negative, the value will be rounded to the left of the decimal point. - If, after rounding, the number of digits to the left of the decimal point - exceeds the declared precision minus the declared scale, an error is - raised. Similarly, if the declared scale exceeds the declared precision - and the number of zero digits to the right of the decimal point is less - than the declared scale minus the declared precision, an error is raised. + number of fractional digits. Then, if the number of digits to the + left of the decimal point exceeds the declared precision minus the + declared scale, an error is raised. For example, a column declared as NUMERIC(3, 1) - will round values to 1 decimal place and be able to store values between - -99.9 and 99.9, inclusive. A column declared as + will round values to 1 decimal place and can store values between + -99.9 and 99.9, inclusive. + + + + Beginning in PostgreSQL 15, it is allowed + to declare a numeric column with a negative scale. Then + values will be rounded to the left of the decimal point. The + precision still represents the maximum number of non-rounded digits. + Thus, a column declared as NUMERIC(2, -3) - will round values to the nearest thousand and be able to store values - between -99000 and 99000, inclusive. A column declared as + will round values to the nearest thousand and can store values + between -99000 and 99000, inclusive. + It is also allowed to declare a scale larger than the declared + precision. Such a column can only hold fractional values, and it + requires the number of zero digits just to the right of the decimal + point to be at least the declared scale minus the declared precision. + For example, a column declared as NUMERIC(3, 5) - will round values to 5 decimal places and be able to store values between + will round values to 5 decimal places and can store values between -0.00999 and 0.00999, inclusive. - The scale in a NUMERIC type declaration may be any value in - the range -1000 to 1000. (The SQL standard requires - the scale to be in the range 0 to precision. - Using values outside this range may not be portable to other database - systems.) + PostgreSQL permits the scale in + a numeric type declaration to be any value in the range + -1000 to 1000. However, the SQL standard requires + the scale to be in the range 0 + to precision. Using scales outside that + range may not be portable to other database systems. diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 46cb37cea1..faff09f5d5 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -827,21 +827,31 @@ numeric_is_integral(Numeric num) * * For purely historical reasons VAR
Re: WIP: Relaxing the constraints on numeric scale
On Wed, 21 Jul 2021 at 22:33, Tom Lane wrote: > > I took a brief look at this and have a couple of quick suggestions: > Thanks for looking at this! > * As you mention, keeping some spare bits in the typmod might come > in handy some day, but as given this patch isn't really doing so. > I think it might be advisable to mask the scale off at 11 bits, > preserving the high 5 bits of the low-order half of the word for future > use. The main objection to that I guess is that it would complicate > doing sign extension in TYPMOD_SCALE(). But it doesn't seem like we > use that logic in any really hot code paths, so another instruction > or three probably is not much of a cost. > Yeah, that makes sense, and it's worth documenting where the spare bits are. Interestingly, gcc recognised the bit hack I used for sign extension and turned it into (x << 21) >> 21 using x86 shl and sar instructions, though I didn't write it that way because apparently that's not portable. > * I agree with wrapping the typmod construction/extraction into macros > (or maybe they should be inline functions?) but the names you chose > seem generic enough to possibly confuse onlookers. I'd suggest > changing TYPMOD to NUMERIC_TYPMOD or NUM_TYPMOD. The comment for them > should probably also explicitly explain "For purely historical reasons, > VARHDRSZ is added to the typmod value after these fields are combined", > or words to that effect. > I've turned them into inline functions, since that makes them easier to read, and debug if necessary. All your other suggestions make sense too. Attached is a new version. Regards, Dean diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml new file mode 100644 index e016f96..6abda2f --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -545,8 +545,8 @@ NUMERIC(precision, scale) - The precision must be positive, the scale zero or positive. - Alternatively: + The precision must be positive, the scale may be positive or negative + (see below). Alternatively: NUMERIC(precision) @@ -578,11 +578,41 @@ NUMERIC If the scale of a value to be stored is greater than the declared scale of the column, the system will round the value to the specified - number of fractional digits. Then, if the number of digits to the - left of the decimal point exceeds the declared precision minus the - declared scale, an error is raised. + number of fractional digits. If the declared scale of the column is + negative, the value will be rounded to the left of the decimal point. + If, after rounding, the number of digits to the left of the decimal point + exceeds the declared precision minus the declared scale, an error is + raised. Similarly, if the declared scale exceeds the declared precision + and the number of zero digits to the right of the decimal point is less + than the declared scale minus the declared precision, an error is raised. + For example, a column declared as + +NUMERIC(3, 1) + + will round values to 1 decimal place and be able to store values between + -99.9 and 99.9, inclusive. A column declared as + +NUMERIC(2, -3) + + will round values to the nearest thousand and be able to store values + between -99000 and 99000, inclusive. A column declared as + +NUMERIC(3, 5) + + will round values to 5 decimal places and be able to store values between + -0.00999 and 0.00999, inclusive. + + + The scale in a NUMERIC type declaration may be any value in + the range -1000 to 1000. (The SQL standard requires + the scale to be in the range 0 to precision. + Using values outside this range may not be portable to other database + systems.) + + + Numeric values are physically stored without any extra leading or trailing zeroes. Thus, the declared precision and scale of a column diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c new file mode 100644 index 2a0f68f..46cb37c --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -816,6 +816,52 @@ numeric_is_integral(Numeric num) } /* + * make_numeric_typmod() - + * + * Pack numeric precision and scale values into a typmod. The upper 16 bits + * are used for the precision (though actually not all these bits are needed, + * since the maximum allowed precision is 1000). The lower 16 bits are for + * the scale, but since the scale is constrained to the range [-1000, 1000], + * we use just the lower 11 of those 16 bits, and leave the remaining 5 bits + * unset, for possible future use. + * + * For purely historical reasons VARHDRSZ is then added to the result, thus + * the unused space in the upper 16 bits is not all as freely available as it + * might seem. + */ +static inline int +make_numeric_typmod(int precision, int scale) +{ + return ((precision << 16) | (scale & 0x7ff)) + VARHDRSZ; +} + +/* + * numeric_t
Re: WIP: Relaxing the constraints on numeric scale
Dean Rasheed writes: > Attached is a more complete patch, with updated docs and tests. I took a brief look at this and have a couple of quick suggestions: * As you mention, keeping some spare bits in the typmod might come in handy some day, but as given this patch isn't really doing so. I think it might be advisable to mask the scale off at 11 bits, preserving the high 5 bits of the low-order half of the word for future use. The main objection to that I guess is that it would complicate doing sign extension in TYPMOD_SCALE(). But it doesn't seem like we use that logic in any really hot code paths, so another instruction or three probably is not much of a cost. * I agree with wrapping the typmod construction/extraction into macros (or maybe they should be inline functions?) but the names you chose seem generic enough to possibly confuse onlookers. I'd suggest changing TYPMOD to NUMERIC_TYPMOD or NUM_TYPMOD. The comment for them should probably also explicitly explain "For purely historical reasons, VARHDRSZ is added to the typmod value after these fields are combined", or words to that effect. * It might be advisable to write NUMERIC_MIN_SCALE with parens: #define NUMERIC_MIN_SCALE (-1000) to avoid any precedence gotchas. * I'd be inclined to leave the num_typemod_test table in place, rather than dropping it, so that it serves to exercise pg_dump for these cases during the pg_upgrade test. Haven't read the code in detail yet. regards, tom lane
Re: WIP: Relaxing the constraints on numeric scale
Attached is a more complete patch, with updated docs and tests. I chose to allow the scale to be in the range -1000 to 1000, which, to some extent, is quite arbitrary. The upper limit of 1000 makes sense, because nearly all numeric computations (other than multiply, add and subtract) have that as their upper scale limit (that's the maximum display scale). It also has to be at least 1000 for SQL compliance, since the precision can be up to 1000. The lower limit, on the other hand, really is quite arbitrary. -1000 is a nice round number, giving it a certain symmetry, and is almost certainly sufficient for any realistic use case (-1000 means numbers are rounded to the nearest multiple of 10^1000). Also, keeping some spare bits in the typemod might come in handy one day for something else (e.g., rounding mode choice). Regards, Dean diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml new file mode 100644 index c473d6a..97e4cdf --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -545,8 +545,8 @@ NUMERIC(precision, scale) - The precision must be positive, the scale zero or positive. - Alternatively: + The precision must be positive, the scale may be positive or negative + (see below). Alternatively: NUMERIC(precision) @@ -578,11 +578,41 @@ NUMERIC If the scale of a value to be stored is greater than the declared scale of the column, the system will round the value to the specified - number of fractional digits. Then, if the number of digits to the - left of the decimal point exceeds the declared precision minus the - declared scale, an error is raised. + number of fractional digits. If the declared scale of the column is + negative, the value will be rounded to the left of the decimal point. + If, after rounding, the number of digits to the left of the decimal point + exceeds the declared precision minus the declared scale, an error is + raised. Similarly, if the declared scale exceeds the declared precision + and the number of zero digits to the right of the decimal point is less + than the declared scale minus the declared precision, an error is raised. + For example, a column declared as + +NUMERIC(3, 1) + + will round values to 1 decimal place and be able to store values between + -99.9 and 99.9, inclusive. A column declared as + +NUMERIC(2, -3) + + will round values to the nearest thousand and be able to store values + between -99000 and 99000, inclusive. A column declared as + +NUMERIC(3, 5) + + will round values to 5 decimal places and be able to store values between + -0.00999 and 0.00999, inclusive. + + + The scale in a NUMERIC type declaration may be any value in + the range -1000 to 1000. (The SQL standard requires + the scale to be in the range 0 to precision. + Using values outside this range may not be portable to other database + systems.) + + + Numeric values are physically stored without any extra leading or trailing zeroes. Thus, the declared precision and scale of a column diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c new file mode 100644 index eb78f0b..2001d75 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -250,6 +250,17 @@ struct NumericData | ((n)->choice.n_short.n_header & NUMERIC_SHORT_WEIGHT_MASK)) \ : ((n)->choice.n_long.n_weight)) +/* + * Pack the numeric precision and scale in the typmod value. The upper 16 + * bits are used for the precision, and the lower 16 bits for the scale. Note + * that the scale may be negative, so use sign extension when unpacking it. + */ + +#define MAKE_TYPMOD(p, s) p) << 16) | ((s) & 0x)) + VARHDRSZ) + +#define TYPMOD_PRECISION(t) t) - VARHDRSZ) >> 16) & 0x) +#define TYPMOD_SCALE(t) ((int32) ((int16) (((t) - VARHDRSZ) & 0x))) + /* -- * NumericVar is the format we use for arithmetic. The digit-array part * is the same as the NumericData storage format, but the header is more @@ -826,7 +837,7 @@ numeric_maximum_size(int32 typmod) return -1; /* precision (ie, max # of digits) is in upper bits of typmod */ - precision = ((typmod - VARHDRSZ) >> 16) & 0x; + precision = TYPMOD_PRECISION(typmod); /* * This formula computes the maximum number of NumericDigits we could need @@ -1080,10 +1091,10 @@ numeric_support(PG_FUNCTION_ARGS) Node *source = (Node *) linitial(expr->args); int32 old_typmod = exprTypmod(source); int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue); - int32 old_scale = (old_typmod - VARHDRSZ) & 0x; - int32 new_scale = (new_typmod - VARHDRSZ) & 0x; - int32 old_precision = (old_typmod - VARHDRSZ) >> 16 & 0x; - int32 new_precision = (new_typmod - VARHDRSZ) >> 16 & 0x; + int32 old_scale = TYPMOD_SCALE(old_typmod); + int32 new_scale = TYPMOD_SCAL
Re: WIP: Relaxing the constraints on numeric scale
On Tue, Jun 29, 2021 at 4:46 PM Dean Rasheed wrote: > On Tue, 29 Jun 2021 at 21:34, Robert Haas wrote: > > I thought about this too, but > > http://postgr.es/m/774767.1591985...@sss.pgh.pa.us made me think that > > it would be an on-disk format break. Maybe it's not, though? > > No, because the numeric dscale remains non-negative, so there's no > change to the way numeric values are stored. The only change is to > extend the allowed scale in the numeric typemod. Ah! Well, in that case, this sounds great. (I haven't looked at the patch, so this is just an endorsement of the concept.) -- Robert Haas EDB: http://www.enterprisedb.com
Re: WIP: Relaxing the constraints on numeric scale
Robert Haas writes: > On Tue, Jun 29, 2021 at 3:58 PM Dean Rasheed wrote: >> When specifying NUMERIC(precision, scale) the scale is constrained to >> the range [0, precision], which is per SQL spec. However, at least one >> other major database vendor intentionally does not impose this >> restriction, since allowing scales outside this range can be useful. > I thought about this too, but > http://postgr.es/m/774767.1591985...@sss.pgh.pa.us made me think that > it would be an on-disk format break. Maybe it's not, though? See further down in that thread --- I don't think there's actually a need for negative dscale on-disk. However, there remains the question of whether any external code knows enough about numeric typmods to become confused by a negative scale field within those. After reflecting for a bit, I suspect the answer is "probably", but it seems like it wouldn't be much worse of an update than any number of other catalog changes we make every release. regards, tom lane
Re: WIP: Relaxing the constraints on numeric scale
On Tue, 29 Jun 2021 at 21:34, Robert Haas wrote: > > I thought about this too, but > http://postgr.es/m/774767.1591985...@sss.pgh.pa.us made me think that > it would be an on-disk format break. Maybe it's not, though? > No, because the numeric dscale remains non-negative, so there's no change to the way numeric values are stored. The only change is to extend the allowed scale in the numeric typemod. Regards, Dean
Re: WIP: Relaxing the constraints on numeric scale
On Tue, Jun 29, 2021 at 3:58 PM Dean Rasheed wrote: > When specifying NUMERIC(precision, scale) the scale is constrained to > the range [0, precision], which is per SQL spec. However, at least one > other major database vendor intentionally does not impose this > restriction, since allowing scales outside this range can be useful. I thought about this too, but http://postgr.es/m/774767.1591985...@sss.pgh.pa.us made me think that it would be an on-disk format break. Maybe it's not, though? -- Robert Haas EDB: http://www.enterprisedb.com
WIP: Relaxing the constraints on numeric scale
When specifying NUMERIC(precision, scale) the scale is constrained to the range [0, precision], which is per SQL spec. However, at least one other major database vendor intentionally does not impose this restriction, since allowing scales outside this range can be useful. A negative scale implies rounding before the decimal point. For example, a column declared as NUMERIC(3,-3) rounds values to the nearest thousand, and can hold values up to 999000. (Note that the display scale remains non-negative, so all digits before the decimal point are displayed, and none of the internals of numeric.c need to worry about negative dscale values. Only the scale in the typemod is negative.) A scale greater than the precision constrains the value to be less than 0.1. For example, a column declared as NUMERIC(3,6) can hold "micro" quantities up to 0.000999. Attached is a WIP patch supporting this. Regards, Dean diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml new file mode 100644 index de561cd..1777c41 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -545,7 +545,7 @@ NUMERIC(precision, scale) - The precision must be positive, the scale zero or positive. + The precision must be positive. Alternatively: NUMERIC(precision) @@ -578,9 +578,28 @@ NUMERIC If the scale of a value to be stored is greater than the declared scale of the column, the system will round the value to the specified - number of fractional digits. Then, if the number of digits to the - left of the decimal point exceeds the declared precision minus the - declared scale, an error is raised. + number of fractional digits. A negative scale may be specified, to round + values to the left of the decimal point. The maximum absolute value + allowed in the column is determined by the declared precision minus the + declared scale. For example, a column declared as + NUMERIC(3, 1) can hold values between -99.9 and 99.9, + inclusive. If the value to be stored exceeds these limits, an error is + raised. + + + + If the declared scale of the column is negative, stored values will be + rounded to the left of the decimal point. For example, a column declared + as NUMERIC(2, -3) will round values to the nearest + thousand and can store values between -99000 and 99000, inclusive. + + + + If the declared scale of the column is greater than or equal to the + declared precision, stored values must only contain fractional digits to + the right of the decimal point. For example, a column declared as + NUMERIC(3, 5) can hold values between -0.00999 and + 0.00999, inclusive. diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c new file mode 100644 index eb78f0b..2001d75 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -250,6 +250,17 @@ struct NumericData | ((n)->choice.n_short.n_header & NUMERIC_SHORT_WEIGHT_MASK)) \ : ((n)->choice.n_long.n_weight)) +/* + * Pack the numeric precision and scale in the typmod value. The upper 16 + * bits are used for the precision, and the lower 16 bits for the scale. Note + * that the scale may be negative, so use sign extension when unpacking it. + */ + +#define MAKE_TYPMOD(p, s) p) << 16) | ((s) & 0x)) + VARHDRSZ) + +#define TYPMOD_PRECISION(t) t) - VARHDRSZ) >> 16) & 0x) +#define TYPMOD_SCALE(t) ((int32) ((int16) (((t) - VARHDRSZ) & 0x))) + /* -- * NumericVar is the format we use for arithmetic. The digit-array part * is the same as the NumericData storage format, but the header is more @@ -826,7 +837,7 @@ numeric_maximum_size(int32 typmod) return -1; /* precision (ie, max # of digits) is in upper bits of typmod */ - precision = ((typmod - VARHDRSZ) >> 16) & 0x; + precision = TYPMOD_PRECISION(typmod); /* * This formula computes the maximum number of NumericDigits we could need @@ -1080,10 +1091,10 @@ numeric_support(PG_FUNCTION_ARGS) Node *source = (Node *) linitial(expr->args); int32 old_typmod = exprTypmod(source); int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue); - int32 old_scale = (old_typmod - VARHDRSZ) & 0x; - int32 new_scale = (new_typmod - VARHDRSZ) & 0x; - int32 old_precision = (old_typmod - VARHDRSZ) >> 16 & 0x; - int32 new_precision = (new_typmod - VARHDRSZ) >> 16 & 0x; + int32 old_scale = TYPMOD_SCALE(old_typmod); + int32 new_scale = TYPMOD_SCALE(new_typmod); + int32 old_precision = TYPMOD_PRECISION(old_typmod); + int32 new_precision = TYPMOD_PRECISION(new_typmod); /* * If new_typmod < VARHDRSZ, the destination is unconstrained; @@ -1115,11 +1126,11 @@ numeric (PG_FUNCTION_ARGS) Numeric num = PG_GETARG_NUMERIC(0); int32 typmod = PG_GETARG_INT32(1); Numeric new; - int32 tmp_typmod; int precision; int scale; int ddigits;