Re: using index or check in ALTER TABLE SET NOT NULL
Robert Haas <robertmhaas@gmail.com>
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Sergei Kornilov <sk@zsrv.org>,
David Rowley <david.rowley@2ndquadrant.com>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>,
Alvaro Herrera <alvherre@alvh.no-ip.org>, Stephen Frost <sfrost@snowman.net>, Ildar Musin <i.musin@postgrespro.ru>
Date: 2019-03-13T17:24:29Z
Lists: pgsql-hackers
On Wed, Mar 13, 2019 at 1:09 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Sergei Kornilov <sk@zsrv.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
Commits
-
Avoid using INFO elevel for what are fundamentally debug messages.
- db438318997b 13.0 landed
- 121e3ceed6d9 12.0 landed
-
Revert setting client_min_messages to 'debug1' in new tests.
- 5655565c077c 12.0 landed
-
Allow ALTER TABLE .. SET NOT NULL to skip provably unnecessary scans.
- bbb96c3704c0 12.0 landed
-
Improve predtest.c's internal docs, and enhance its functionality a bit.
- 5748f3a0aa7c 11.0 cited