Numeric 508 datatype

Simon Riggs wrote:

Now we're into 8.2devel mode, its time to submit the previously
discussed patch that:

- reduces Numeric storage format by 2 bytes

This makes the often discussed binary upgrade impossible, so I wonder if
two bytes savings are worth the trouble.

Regards,
Andreas

Andreas Pflug <pgadmin@pse-consulting.de> writes:

Simon Riggs wrote:

Now we're into 8.2devel mode, its time to submit the previously
discussed patch that:
- reduces Numeric storage format by 2 bytes

This makes the often discussed binary upgrade impossible, so I wonder if
two bytes savings are worth the trouble.

Unless someone actually steps forward and produces a working pg_upgrade
in the 8.2 timeframe, this objection is moot.

regards, tom lane

Tom Lane wrote:

Andreas Pflug <pgadmin@pse-consulting.de> writes:

Simon Riggs wrote:

Now we're into 8.2devel mode, its time to submit the previously
discussed patch that:
- reduces Numeric storage format by 2 bytes

This makes the often discussed binary upgrade impossible, so I wonder if
two bytes savings are worth the trouble.

Unless someone actually steps forward and produces a working pg_upgrade
in the 8.2 timeframe, this objection is moot.

Hm, so if this patch is applied now, and in 5 months or so somebody
implements pg_upgrade, this numeric storage patch would be rolled back?
OTOH, an upgrade mechanism that's compatible for future 8.3+ versions
only seems not too attractive.
A solution might be to keep the current numeric implementation under a
different name (deprecatednumeric or so), for backward compatibility
(this should apply to future storage format changes as well).

Regards,
Andreas

Hm, so if this patch is applied now, and in 5 months or so somebody
implements pg_upgrade, this numeric storage patch would be rolled back?
OTOH, an upgrade mechanism that's compatible for future 8.3+ versions
only seems not too attractive.

With Slony and Replicator I don't really see the need for in place
upgrades.

Joshua D. Drake

-----Original Message-----
From: pgsql-patches-owner@postgresql.org
[mailto:pgsql-patches-owner@postgresql.org] On Behalf Of
Joshua D. Drake
Sent: 17 November 2005 15:58
To: Andreas Pflug
Cc: Tom Lane; Simon Riggs; pgsql-patches@postgresql.org
Subject: Re: [PATCHES] Numeric 508 datatype

Hm, so if this patch is applied now, and in 5 months or so somebody
implements pg_upgrade, this numeric storage patch would be

rolled back?

OTOH, an upgrade mechanism that's compatible for future

8.3+ versions

only seems not too attractive.

With Slony and Replicator I don't really see the need for in place
upgrades.

They're not exactly easy to setup for the casual user (well, I assume
replicator isn't but it's not relevant to the majority of our users
anyway).

Regards, Dave.

Joshua D. Drake wrote:

With Slony and Replicator I don't really see the need for in place
upgrades.

Maintaining a replica is hardly a cost-free exercise.

However, I don't think we can promise never to change the ondisk
representation of data, nor the page layout. Sometimes an inplace
upgrade just won't work, ISTM.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

However, I don't think we can promise never to change the ondisk
representation of data, nor the page layout. Sometimes an inplace
upgrade just won't work, ISTM.

We have talked about batching on-disk changes so that they'd only occur
once every few release cycles. But until we have a pg_upgrade, there is
no reason to adopt such a policy.

regards, tom lane

On Thu, 2005-11-17 at 11:20 -0500, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

However, I don't think we can promise never to change the ondisk
representation of data, nor the page layout. Sometimes an inplace
upgrade just won't work, ISTM.

We have talked about batching on-disk changes so that they'd only occur
once every few release cycles. But until we have a pg_upgrade, there is
no reason to adopt such a policy.

If the patch is accepted technically, in can be applied at any time,
right up to code freeze for this or the next release. It's a fairly
independent patch.

I'd suggest we check it out now, then put it in a holding pen for awhile
to see if an upgrade tool emerges.

Best Regards, Simon Riggs

Simon Riggs <simon@2ndquadrant.com> writes:

If the patch is accepted technically, in can be applied at any time,
right up to code freeze for this or the next release. It's a fairly
independent patch.

I'd suggest we check it out now, then put it in a holding pen for awhile
to see if an upgrade tool emerges.

I'm disinclined to do that unless there's a pretty firm commitment from
someone to work on pg_upgrade in the near future. Patches that are not
in the tree tend to suffer from code drift; if we wait six months or a
year to apply what you've done then we'll likely be looking at
significantly more work to get it in. We'd also be losing the direct
and indirect testing that the patch would get were it in the tree over
that length of time.

regards, tom lane

They're not exactly easy to setup for the casual user (well, I assume
replicator isn't but it's not relevant to the majority of our users
anyway).

Well FYI any half way competent person can set up replicator in 30
minutes or less ;), that said
in place upgrades are a feature that will take up a lot of man power
EVERY release I would think.

I could be wrong but I would rather see the resources put into other things.

J

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

However, I don't think we can promise never to change the ondisk
representation of data, nor the page layout. Sometimes an inplace
upgrade just won't work, ISTM.

We have talked about batching on-disk changes so that they'd only occur
once every few release cycles. But until we have a pg_upgrade, there is
no reason to adopt such a policy.

IMHO such a policy is a _prerequisite_ for somebody to come up
implementing pg_upgrade. Why spend time on pg_upgrade if there's no
policy to support it?

Regards,
Andreas

Andreas Pflug wrote:

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

However, I don't think we can promise never to change the ondisk
representation of data, nor the page layout. Sometimes an inplace
upgrade just won't work, ISTM.

We have talked about batching on-disk changes so that they'd only occur
once every few release cycles. But until we have a pg_upgrade, there is
no reason to adopt such a policy.

IMHO such a policy is a _prerequisite_ for somebody to come up
implementing pg_upgrade. Why spend time on pg_upgrade if there's no
policy to support it?

Is anybody working or considering to work on pg_upgrade, or is all this
hypothetical? Our past history has seen lots of people offering to work
on pg_upgrade, and none has produced a working version. Is it fair or
useful to impose restrictions on development just because it's remotely
possible that somebody is going to be motivated enough to consider
producing it?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

Is anybody working or considering to work on pg_upgrade, or is all this
hypothetical? Our past history has seen lots of people offering to work
on pg_upgrade, and none has produced a working version. Is it fair or
useful to impose restrictions on development just because it's remotely
possible that somebody is going to be motivated enough to consider
producing it?

Depends on the impact the restriction imposes. If
stability/scalability/functionality or so is affected, this sounds not
tolerable. If it's about not saving two bytes that have been spoiled for
ages before, or keeping a backward compatibility type, it appears
feasible to me.
Changing on-disk structures at the start of the 8.2 dev cycle is a
guarantee that nobody will implement pg_upgrade for 8.2.

Regards,
Andreas

Andreas Pflug wrote:

Alvaro Herrera wrote:

Is anybody working or considering to work on pg_upgrade, or is all this
hypothetical? Our past history has seen lots of people offering to work
on pg_upgrade, and none has produced a working version. Is it fair or
useful to impose restrictions on development just because it's remotely
possible that somebody is going to be motivated enough to consider
producing it?

Depends on the impact the restriction imposes. If
stability/scalability/functionality or so is affected, this sounds not
tolerable. If it's about not saving two bytes that have been spoiled for
ages before, or keeping a backward compatibility type, it appears
feasible to me.
Changing on-disk structures at the start of the 8.2 dev cycle is a
guarantee that nobody will implement pg_upgrade for 8.2.

Let's go ahead and apply the patch. While this change isn't very
significant, I bet there will be other changes in 8.2 where we will want
to change the database for a significant benefit, like reducing the
tuple header by 4 bytes by recompressing the four xid/cid fields back
into three.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:

Let's go ahead and apply the patch. While this change isn't very
significant, I bet there will be other changes in 8.2 where we will want
to change the database for a significant benefit, like reducing the
tuple header by 4 bytes by recompressing the four xid/cid fields back
into three.

All of which makes me very uncomfortable about doing 64 bit enums. I am
still trying to think of a better solution.

cheers

andrew

Where are we on this patch? It is ready for the patch queue?

---------------------------------------------------------------------------

Simon Riggs wrote:

Now we're into 8.2devel mode, its time to submit the previously
discussed patch that:

- reduces Numeric storage format by 2 bytes
- limits scale to +/- 508 decimal places

This is sufficient to allow Numeric to continue to be used as the
default numeric representation for all numbers in the parser.

Passes: make check on cvstip, as well as some tests not in there.

Code comments explain the new format and consequences.

As previously agreed, reviewing this is a 2 stage process:
1. review/possibly agree OK to commit
2. check with everybody on GENERAL that the restriction to 508 is
acceptable

Figure there's no point doing (2) until we agree the proposal/code is
workable.

As Atsushi-san point out, there is also come CPU optimization to be done
on Numeric comparison, and also on other areas such as aggregation. I've
not done this yet.

Best Regards, Simon Riggs

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Thu, 2005-12-01 at 23:34 -0500, Bruce Momjian wrote:

Where are we on this patch? It is ready for the patch queue?

It's good to be applied, AFAIK.

Simon Riggs wrote:

As previously agreed, reviewing this is a 2 stage process:
1. review/possibly agree OK to commit
2. check with everybody on GENERAL that the restriction to 508 is
acceptable

Figure there's no point doing (2) until we agree the proposal/code is
workable.

I was hoping you'd give me a "this looks good enough to apply"
thumbs-up, then I'll ask for comments via a Weekly News item.

If I ask for comments and then it is technically rejected we would be
wasting everybody's time.

Best Regards, Simon Riggs

Simon Riggs wrote:

Now we're into 8.2devel mode, its time to submit the previously
discussed patch that:

- reduces Numeric storage format by 2 bytes
- limits scale to +/- 508 decimal places

This is sufficient to allow Numeric to continue to be used as the
default numeric representation for all numbers in the parser.

Passes: make check on cvstip, as well as some tests not in there.

Code comments explain the new format and consequences.

As previously agreed, reviewing this is a 2 stage process:
1. review/possibly agree OK to commit
2. check with everybody on GENERAL that the restriction to 508 is
acceptable

Figure there's no point doing (2) until we agree the proposal/code is
workable.

OK, seems all objections have been dealt with so it goes into the patch
queue. I will ask on 'general'.

The only downside I see is that I can't impress people by doing:

SELECT factorial(4000);

I don't suppose the _impression_ factor is worth two bytes per value.
Shame.

I suppose people wanting to do such manipulations will have to store the
numbers as text and use a server-side library like perl to do
calculations.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:

OK, seems all objections have been dealt with so it goes into the patch
queue. I will ask on 'general'.

The only downside I see is that I can't impress people by doing:

SELECT factorial(4000);

I don't suppose the _impression_ factor is worth two bytes per value.
Shame.

I suppose people wanting to do such manipulations will have to store the
numbers as text and use a server-side library like perl to do
calculations.

Oops, I was wrong about this. The patch changes the maximum _specified_
precision:

/*
! * Hardcoded precision limit - arbitrary, but must be small enough that
! * dscale values will fit in 14 bits.
*/
! #define NUMERIC_MAX_PRECISION 1000

	  /*
	   * Internal limits on the scales chosen for calculation results
	--- 15,23 ----
	  #define _PG_NUMERIC_H_

/*
! * Hardcoded precision limit - maximum that can fit in Numeric storage
*/
! #define NUMERIC_MAX_PRECISION 508

but in fact, our computational precision is 4096, and we silently
overflow for values greater than that:

test=> create table test(x numeric);
CREATE TABLE
test=> insert into test values (factorial(4000));
INSERT 0 1

The length is 4096 digits, and so is factorial(10000) --- clearly wrong.
I now see in the TODO:

* Change NUMERIC to enforce the maximum precision, and increase it

So we are really decreasing the specified precision from 1000 to 508,
and the computational precision from 4096 to 508. Is there any plan to
fix the silent overflow problem? Is that in the patch? I don't see it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073