ALTER TABLE ... ALTER COLUMN ... SET DISTINCT

Robert Haas <robertmhaas@gmail.com> writes:

Per previous discussion.
http://archives.postgresql.org/message-id/8066.1229106059@sss.pgh.pa.us
http://archives.postgresql.org/message-id/603c8f070904021926g92eb55sdfc68141133957c1@mail.gmail.com

I'm not thrilled about adding a column to pg_attribute for this.
Isn't there some way of keeping it in pg_statistic?

regards, tom lane

On Sat, Apr 4, 2009 at 7:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Per previous discussion.
http://archives.postgresql.org/message-id/8066.1229106059@sss.pgh.pa.us
http://archives.postgresql.org/message-id/603c8f070904021926g92eb55sdfc68141133957c1@mail.gmail.com

I'm not thrilled about adding a column to pg_attribute for this.
Isn't there some way of keeping it in pg_statistic?

I don't like the idea of keeping it in pg_statistic. Right now, all
of the data in pg_statistic is transient, so you could theoretically
truncate the table at any time without losing anything permanent.
It's true that we don't do that right now, but it seems cleaner to
keep the data generated by the analyzer separate from the stuff we
consider part of the structure of the database. If we did put the
data in pg_statistic, then we'd have to teach vacuum that when it
writes out new statistics, it also has to copy over this setting from
the previous version of the tuple. And that means it would have to
lock the tuples against concurrent updates while analyze is running.
Also, if someone happened to run ALTER TABLE SET DISTINCT before the
first run of ANALYZE on that table (for example, during pg_load)
there'd be no existing row in pg_statistic for the DDL command to
update, so we'd need to create and insert a fake row (which,
incidentally, would blow up any concurrent ANALYZE already in progress
when it got and tried to insert the resulting rows into pg_statistic,
violating the unique constraint). All in all it seems rather messy.

What is the specific nature of your concern? I thought about the
possibility of a distributed performance penalty that might be
associated with enlarging pg_attribute, but increasing the size of a
structure that is already 112 bytes by another 4 doesn't seem likely
to be significant, especially since we're not crossing a power-of-two
boundary. It might be possible to reclaim 4 bytes by changing
attstattarget and attndims from int4 to int2, but I'd rather do that
as a separate patch.

...Robert

Robert Haas escribi�:

On Sat, Apr 4, 2009 at 7:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Per previous discussion.
http://archives.postgresql.org/message-id/8066.1229106059@sss.pgh.pa.us
http://archives.postgresql.org/message-id/603c8f070904021926g92eb55sdfc68141133957c1@mail.gmail.com

I'm not thrilled about adding a column to pg_attribute for this.
Isn't there some way of keeping it in pg_statistic?

I don't like the idea of keeping it in pg_statistic. Right now, all
of the data in pg_statistic is transient, so you could theoretically
truncate the table at any time without losing anything permanent.

Maybe use a new catalog?

What is the specific nature of your concern? I thought about the
possibility of a distributed performance penalty that might be
associated with enlarging pg_attribute, but increasing the size of a
structure that is already 112 bytes by another 4 doesn't seem likely
to be significant, especially since we're not crossing a power-of-two
boundary.

FWIW it has been said that whoever is concerned about pg_attribute bloat
should be first looking at getting rid of the redundant entries for
system columns, for each and every table.

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

Alvaro Herrera <alvherre@commandprompt.com> writes:

FWIW it has been said that whoever is concerned about pg_attribute bloat
should be first looking at getting rid of the redundant entries for
system columns, for each and every table.

That's been overtaken by events, unfortunately: we now need those
entries to carry per-column permissions on system columns. So they're
no longer merely overhead.

Possibly we could hack things so that only system columns with
non-default permissions are actually stored, but that feels like
a kluge.

regards, tom lane

On Sat, Apr 4, 2009 at 10:31 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

Robert Haas escribió:

On Sat, Apr 4, 2009 at 7:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Per previous discussion.
http://archives.postgresql.org/message-id/8066.1229106059@sss.pgh.pa.us
http://archives.postgresql.org/message-id/603c8f070904021926g92eb55sdfc68141133957c1@mail.gmail.com

I'm not thrilled about adding a column to pg_attribute for this.
Isn't there some way of keeping it in pg_statistic?

I don't like the idea of keeping it in pg_statistic.  Right now, all
of the data in pg_statistic is transient, so you could theoretically
truncate the table at any time without losing anything permanent.

Maybe use a new catalog?

If we go that route, we would probably make sense to move
attstattarget there as well. Obviously it wouldn't make sense to move
anything that's in the critical path of ordinary database operations,
but maybe attislocal or attinhcount could be moved as well. But I'm
not sure it's really warranted because, AFAIK, we have no evidence
that this is a real as opposed to a theoretical problem, and even if
we moved all of that stuff, that's only 12 bytes, and now you have
another table that's competing for space in the system cache. If
someone could demonstrate (say, by reducing NAMEDATALEN) that a
smaller pg_attribute structure would generate a real performance
benefit, then it would be worth spending the time to figure out a way
to make that happen (obviously without actually reducing NAMEDATALEN,
that's only a possible way to measure the impact).

What is the specific nature of your concern?  I thought about the
possibility of a distributed performance penalty that might be
associated with enlarging pg_attribute, but increasing the size of a
structure that is already 112 bytes by another 4 doesn't seem likely
to be significant, especially since we're not crossing a power-of-two
boundary.

FWIW it has been said that whoever is concerned about pg_attribute bloat
should be first looking at getting rid of the redundant entries for BN
system columns, for each and every table.

That's a different kind of bloat (more rows vs. larger rows) but a
valid point all the same. I suspect neither type has much practical
impact, and that if we listed all the performance problems that
PostgreSQL has today, neither would be in the top 500. Bad ndistinct
estimates would be, however.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Apr 4, 2009 at 7:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not thrilled about adding a column to pg_attribute for this.

What is the specific nature of your concern?

Actually, I'm more worried about the TupleDesc data structure than
the catalogs. There are TupleDescs all over the backend, and I've
seen evidence in profiles that setting them up is a nontrivial cost.

You're very possibly right that four more bytes is in the noise,
though.

Two other comments now that I've read a little further:

* This isn't happening for 8.4, so adjust the pg_dump code.

* Using an integer is bogus. Use a float4 and forget the weird scaling;
it should have exactly the same interpretation as stadistinct, except
for 0 meaning "unset" instead of "unknown".

regards, tom lane

On Sat, Apr 4, 2009 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Apr 4, 2009 at 7:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not thrilled about adding a column to pg_attribute for this.

What is the specific nature of your concern?

Actually, I'm more worried about the TupleDesc data structure than
the catalogs.  There are TupleDescs all over the backend, and I've
seen evidence in profiles that setting them up is a nontrivial cost.

You're very possibly right that four more bytes is in the noise,
though.

Two other comments now that I've read a little further:

* This isn't happening for 8.4, so adjust the pg_dump code.

I thought about writing 80500, but the effect of that would have been
to render the patch impossible to test, so I didn't. :-)

I think I'll be very lucky if that's the most bitrot this accumulates
between now and when the tree is open for 8.5 development. System
catalog changes stink in that regard. I suppose we could tag and
branch the tree now, but that would just move the work of fixing any
subsequent conflicts from patch authors to committers, which is sort
of a zero-sum game.

* Using an integer is bogus.  Use a float4 and forget the weird scaling;
it should have exactly the same interpretation as stadistinct, except
for 0 meaning "unset" instead of "unknown".

I think there's a pretty good chance that will lead to a complaint
that is some variant of the following: "I ran this command and then I
did a pg_dump and the output doesn't match what I put in." Or maybe,
"I did a dump and a restore on a different machine with a different
architecture and then another dump and then I diffed them and this
popped out."

I have a deep-seated aversion to storing important values as float,
and we seem to have no other floats anywhere in our DDL, so I was a
little leery about breaking new ground. There's nothing particularly
special about the scaling that the pg_statistic stuff uses, and it's
basically pretty obscure internal stuff anyway, so I think the
consistency argument is fairly weak.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Apr 4, 2009 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

* Using an integer is bogus. �Use a float4 and forget the weird scaling;
it should have exactly the same interpretation as stadistinct, except
for 0 meaning "unset" instead of "unknown".

I have a deep-seated aversion to storing important values as float,

[ shrug... ] Precision is not important for this value: we are not
anywhere near needing more than six significant digits for our
statistical estimates. Range, on the other hand, could be important
when dealing with really large tables. So I'm much more concerned
about whether the definition is too restrictive than about whether
some uninformed person complains about exactness.

regards, tom lane

On Sun, Apr 5, 2009 at 7:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Apr 4, 2009 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

* Using an integer is bogus.  Use a float4 and forget the weird scaling;
it should have exactly the same interpretation as stadistinct, except
for 0 meaning "unset" instead of "unknown".

I have a deep-seated aversion to storing important values as float,

[ shrug... ]  Precision is not important for this value: we are not
anywhere near needing more than six significant digits for our
statistical estimates.  Range, on the other hand, could be important
when dealing with really large tables.  So I'm much more concerned
about whether the definition is too restrictive than about whether
some uninformed person complains about exactness.

I thought about that, and if you think that's better, I can implement
it that way. Personally, I'm unconvinced. The use case for
specifying a number of distinct values in excess of 2 billion as an
absolute number rather than as a percentage of the table size seems
pretty weak to me. I would rather use integers and have it be clean.
But I would rather have it your way than not have it at all.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Sun, Apr 5, 2009 at 7:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[ shrug... ] �Precision is not important for this value: we are not
anywhere near needing more than six significant digits for our
statistical estimates. �Range, on the other hand, could be important
when dealing with really large tables.

I thought about that, and if you think that's better, I can implement
it that way. Personally, I'm unconvinced. The use case for
specifying a number of distinct values in excess of 2 billion as an
absolute number rather than as a percentage of the table size seems
pretty weak to me.

I was more concerned about the other end of it. Your patch sets a
not-too-generous lower bound on the percentage that can be represented ...

regards, tom lane

On Sun, Apr 5, 2009 at 10:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sun, Apr 5, 2009 at 7:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[ shrug... ]  Precision is not important for this value: we are not
anywhere near needing more than six significant digits for our
statistical estimates.  Range, on the other hand, could be important
when dealing with really large tables.

I thought about that, and if you think that's better, I can implement
it that way.  Personally, I'm unconvinced.  The use case for
specifying a number of distinct values in excess of 2 billion as an
absolute number rather than as a percentage of the table size seems
pretty weak to me.

I was more concerned about the other end of it.  Your patch sets a
not-too-generous lower bound on the percentage that can be represented ...

Huh? With a scaling factor of 1 million, you can represent anything
down to about 0.000001, which is apparently all you can expect out of
a float4 anyway.

http://archives.postgresql.org/pgsql-bugs/2009-01/msg00039.php

In fact, we could change the scaling factor to 1 billion if you like,
and it would then give you MORE significant digits than you'll get out
of a float4 (and you'll be able to predict the exact number that
you're gonna get). If someone has billions of rows in the table but
only thousands of distinct values, I would expect them to run a script
to count 'em up and specify the exact number, rather than specifying
some microscopic percentage. But there's certainly enough range in
int4 to tack on three more decimal places if you think it's warranted.

(It's also worth pointing out that the calculations we do with
ndistinct are pretty approximations anyway. If the difference between
stadistinct = -1 x 10^-6 and stadistinct = -1.4^10-6 is the thing
that's determining whether the planner is picking the correct plan on
your 4-billion-row table, you probably want to tune some other
parameter as well so as to get further away from that line. Just
getting the value in the ballpark should be a big improvement over how
things stand now.)

...Robert

On Sun, Apr 5, 2009 at 10:38 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sun, Apr 5, 2009 at 10:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sun, Apr 5, 2009 at 7:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[ shrug... ]  Precision is not important for this value: we are not
anywhere near needing more than six significant digits for our
statistical estimates.  Range, on the other hand, could be important
when dealing with really large tables.

I thought about that, and if you think that's better, I can implement
it that way.  Personally, I'm unconvinced.  The use case for
specifying a number of distinct values in excess of 2 billion as an
absolute number rather than as a percentage of the table size seems
pretty weak to me.

I was more concerned about the other end of it.  Your patch sets a
not-too-generous lower bound on the percentage that can be represented ...

Huh?  With a scaling factor of 1 million, you can represent anything
down to about 0.000001, which is apparently all you can expect out of
a float4 anyway.

http://archives.postgresql.org/pgsql-bugs/2009-01/msg00039.php

I guess I'm wrong here - 0.00001 is only one SIGNIFICANT digit. But
the point remains that specifying ndistinct in ppm is probably enough
for most cases, and ppb (which would still fit in int4) even more so.
I don't think we need to worry about people with trillions of rows
(and even they could still specify an absolute number).

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

(It's also worth pointing out that the calculations we do with
ndistinct are pretty approximations anyway. If the difference between
stadistinct = -1 x 10^-6 and stadistinct = -1.4^10-6 is the thing
that's determining whether the planner is picking the correct plan on
your 4-billion-row table,

No, it's the loss of ability to set stadistinct to -1e-9 or -1e-12 or
-1e-15 or so that is bothering me. In a table with billions of rows
that could become important.

Or maybe not; but the real bottom line here is that it is 100% silly to
use a different representation in this column than is used in the
underlying stadistinct column. All you accomplish by that is to impose
on the user the intersection of the accuracy/range limits of the two
different representations.

regards, tom lane

On Sun, Apr 5, 2009 at 11:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

(It's also worth pointing out that the calculations we do with
ndistinct are pretty approximations anyway.  If the difference between
stadistinct = -1 x 10^-6 and stadistinct = -1.4^10-6 is the thing
that's determining whether the planner is picking the correct plan on
your 4-billion-row table,

No, it's the loss of ability to set stadistinct to -1e-9 or -1e-12 or
-1e-15 or so that is bothering me.  In a table with billions of rows
that could become important.

Or maybe not; but the real bottom line here is that it is 100% silly to
use a different representation in this column than is used in the
underlying stadistinct column.  All you accomplish by that is to impose
on the user the intersection of the accuracy/range limits of the two
different representations.

Well, I think I was pretty clear about what I was trying to
accomplish. I think there are more people who care about pg_dump
output being diffable than there are who need to set ndistinct more
accurately than 1 ppm and yet not as an integer. Perhaps if any of
those people are reading this thread they could chime in. Otherwise,
I will implement as you propose.

...Robert

* Robert Haas (robertmhaas@gmail.com) wrote:

Well, I think I was pretty clear about what I was trying to
accomplish. I think there are more people who care about pg_dump
output being diffable than there are who need to set ndistinct more
accurately than 1 ppm and yet not as an integer. Perhaps if any of
those people are reading this thread they could chime in. Otherwise,
I will implement as you propose.

I do such diffs pretty often, but I don't think I've *ever* done it on
catalog tables.. Perhaps it'll come up in the future, but I doubt it.

Stephen

On Mon, Apr 6, 2009 at 7:30 AM, Stephen Frost <sfrost@snowman.net> wrote:

* Robert Haas (robertmhaas@gmail.com) wrote:

Well, I think I was pretty clear about what I was trying to
accomplish.  I think there are more people who care about pg_dump
output being diffable than there are who need to set ndistinct more
accurately than 1 ppm and yet not as an integer.  Perhaps if any of
those people are reading this thread they could chime in.  Otherwise,
I will implement as you propose.

I do such diffs pretty often, but I don't think I've *ever* done it on
catalog tables..  Perhaps it'll come up in the future, but I doubt it.

       Stephen

Well the point is when you dump a user table, it will dump this
setting along with it, same as it does now for statistics_target. So
if you diff the DDL you might see differences in rounding. If you
only diff the data, it won't matter unless, as you say, you're dumping
pg_attribute itself.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

Well, I think I was pretty clear about what I was trying to
accomplish. I think there are more people who care about pg_dump
output being diffable than there are who need to set ndistinct more
accurately than 1 ppm and yet not as an integer.

My, you *are* paranoid about float4 aren't you? Once you've stored
a given value, it's always going to dump the same. Diffing different
dumps isn't going to be a problem. I concede that it might look
different from what you put in originally, but we make no effort to
guarantee that for DDL anyway.

(It is true that with pg_dump's default float_extra_digits setting,
outputs from this column might look uglier than they need to.
I don't see anything wrong with having pg_dump forcibly round the
value to five or so digits, though.)

regards, tom lane

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Mon, Apr 6, 2009 at 7:30 AM, Stephen Frost <sfrost@snowman.net> wrote:

I do such diffs pretty often, but I don't think I've *ever* done it on
catalog tables..  Perhaps it'll come up in the future, but I doubt it.

Well the point is when you dump a user table, it will dump this
setting along with it, same as it does now for statistics_target. So
if you diff the DDL you might see differences in rounding. If you
only diff the data, it won't matter unless, as you say, you're dumping
pg_attribute itself.

The rounding when you dump it out is going to be consistant though, is
it not? I mean, you might get a difference between what you try to set
it to and the result that you get from pg_dump, but if you compare one
pg_dump to another done later there shouldn't be any change, right? If
there is an architecture difference then I could maybe see it, but I
thought float-handling was well-defined on systems we run on.

Thanks,

Stephen

On Mon, Apr 6, 2009 at 10:49 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Well, I think I was pretty clear about what I was trying to
accomplish.  I think there are more people who care about pg_dump
output being diffable than there are who need to set ndistinct more
accurately than 1 ppm and yet not as an integer.

My, you *are* paranoid about float4 aren't you?

Yes - perhaps unreasonably so. I've had so many bad experiences with
floating-point arithmetic that I've stopped trying to understand all
of the crazy things that can happen and started just avoiding it like
the plague. Long live numeric!

Once you've stored
a given value, it's always going to dump the same.  Diffing different
dumps isn't going to be a problem.  I concede that it might look
different from what you put in originally, but we make no effort to
guarantee that for DDL anyway.

(It is true that with pg_dump's default float_extra_digits setting,
outputs from this column might look uglier than they need to.
I don't see anything wrong with having pg_dump forcibly round the
value to five or so digits, though.)

So based on this comment and Stephen's remarks, I'm going to assume
that I'm succumbing to a fit of unjustified paranoia and re-implement
as you suggest.

...Robert