Re: using index or check in ALTER TABLE SET NOT NULL

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Sergei Kornilov <sk(at)zsrv(dot)org>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>, Ildar Musin <i(dot)musin(at)postgrespro(dot)ru>
Subject: Re: using index or check in ALTER TABLE SET NOT NULL
Date: 2019-03-13 17:24:29
Message-ID: CA+TgmoY3Gb9ZL8fHVvPyues8XgFoE=jz-Ry+739SYkczME3vhA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 13, 2019 at 1:09 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Sergei Kornilov <sk(at)zsrv(dot)org> writes:
> >> Ugh, I guess so. Or how about changing the message itself to use
> >> INFO, like we already do in QueuePartitionConstraintValidation?
>
> > Fine for me. But year ago this was implemented in my patch and Tom voted against using INFO level for such purpose: https://www.postgresql.org/message-id/1142.1520362313%40sss.pgh.pa.us
>
> What I thought then was that you didn't need the message at all,
> at any debug level. I still think that. It might have been useful
> for development purposes but it does not belong in committed code.
> INFO (making it impossible for anybody to not have the message
> in-their-face) is right out.

I find that position entirely wrong-headed. If you think users have
no interest in a message telling them whether their gigantic table is
getting scanned or not, you're wrong. Maybe you're right that
everyone doesn't want it, but I'm positive some do. We've had
requests for very similar things on this very mailing list more than
one.

And if you think that it's not useful to have regression tests that
verify that the behavior is correct, well, that's not a question with
one objectively right answer, but I emphatically disagree with that
position. This behavior is often quite subtle, especially in
combination with partitioned tables where different partitions may get
different treatment. It would not be at all difficult to break it
without realizing.

I do understand that we seem to have no good way of doing this that
lets users have the option of seeing this message and also makes it
something that can be regression-tested. INFO is out because there's
no option, and DEBUG1 is out because it doesn't work in the regression
tests. However, I think giving up and saying "ok, well that's just
how it is, too bad" is, one, letting the tail wag the dog, and two, a
terribly disappointing attitude.

I've just reverted the 0002 portion of the patch, which should
hopefully fix this problem for now. But I strongly encourage you
think of something to which you could say "yes" instead of just
shooting everything people want to do in this area down.

--
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 Fabien COELHO 2019-03-13 17:25:08 Re: Timeout parameters
Previous Message Tom Lane 2019-03-13 17:19:00 Re: using index or check in ALTER TABLE SET NOT NULL