Date: Wed, 13 Mar 2019 15:36:50 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>
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>
Subject: Re: using index or check in ALTER TABLE SET NOT NULL
Message-ID: <20190313203650.GS23597@telsasoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoZMDn_9xHBeJVSsgLrRpDk85y_Gp=ccts9x1CHaSb3DKA@mail.gmail.com>
 <7254.1552496972@sss.pgh.pa.us>
User-Agent: Mutt/1.5.23 (2014-03-12)
Precedence: bulk

Hi,

On Wed, Mar 13, 2019 at 01:25:11PM -0400, Robert Haas wrote:
> On Wed, Mar 13, 2019 at 1:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Oh, and yes, I think QueuePartitionConstraintValidation's usage
> > is an unacceptable abuse of INFO level.  I'm surprised we haven't
> > gotten complaints about it yet.
> 
> Perhaps that's because users aren't as direly opposed to informational
> messages from DDL commands as you seem to believe...

On Wed, Mar 13, 2019 at 01:09:32PM -0400, Tom Lane 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'm writing to provide a datapoint for the existing message regarding "implied
by existing constraints" in QueuePartitionConstraintValidation.

On the one hand, I agree that message at INFO is more verbose than other
commands, and I'm 20% surprised by it.

On the other hand, I *like* the output (including at INFO level).  It alerted
me to 1) deficiency with some of our tables (due to column not marked NOT
NULL); and, 2) helped me to understand how to improve some of our dynamic DDL.

If the message weren't visible at LOG, I'd miss it, at least sometimes, and
look for how to re-enable it (but statement logging of DEBUG is beyond
excessive).

For context, I'm someone who configures servers with log_min_messages=info and
who sometimes SETs client_min_messages='DEBUG1' ; I have $toomany warnings in
our monitoring system, because I'd rather see twice as much stuff that may or
may not be interesting than miss 10% of things I needed to see.

Justin