-DDISABLE_ENABLE_ASSERT

Andres Freund <andres@2ndquadrant.com> writes:

Since there seem to be multiple static checkers (coverity, clang
checker) having problems with assert_enabled can we just optionally
disable it?
I am thinking of replacing it by a AssertionsEnabled() macro which then
is unconditionally defined when DISABLE_ENABLE_ASSERT is defined.

We could do that ... but I wonder if we shouldn't remove assert_enabled
altogether. What's the use case for turning it off? Not matching the
speed of a non-cassert build, because for instance MEMORY_CONTEXT_CHECKING
doesn't get turned off.

If we went this direction I'd suggest keeping the GUC but turning it into
a read-only report of whether the backend was compiled with assertions.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, May 22, 2014 at 4:31 PM, Andres Freund <andres@2ndquadrant.com> wrote:

Since there seem to be multiple static checkers (coverity, clang
checker) having problems with assert_enabled can we just optionally
disable it?
I am thinking of replacing it by a AssertionsEnabled() macro which then
is unconditionally defined when DISABLE_ENABLE_ASSERT is defined.

Uh, probably. But I think you need a better name; specifically, one
that doesn't contain two words which are antonyms.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-05-22 16:37:35 -0400, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

Since there seem to be multiple static checkers (coverity, clang
checker) having problems with assert_enabled can we just optionally
disable it?
I am thinking of replacing it by a AssertionsEnabled() macro which then
is unconditionally defined when DISABLE_ENABLE_ASSERT is defined.

We could do that ... but I wonder if we shouldn't remove assert_enabled
altogether. What's the use case for turning it off? Not matching the
speed of a non-cassert build, because for instance MEMORY_CONTEXT_CHECKING
doesn't get turned off.

I've used it once or twice to avoid having to recompile postgres when I
wanted things not to be *that* slow (AtEOXactBuffers() I am looking at
you). But I wouldn't be very sad if it'd go.

Anybody against that?

If we went this direction I'd suggest keeping the GUC but turning it into
a read-only report of whether the backend was compiled with assertions.

Yes, that makes sense.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund wrote:

On 2014-05-22 16:37:35 -0400, Tom Lane wrote:

We could do that ... but I wonder if we shouldn't remove assert_enabled
altogether. What's the use case for turning it off? Not matching the
speed of a non-cassert build, because for instance MEMORY_CONTEXT_CHECKING
doesn't get turned off.

I've used it once or twice to avoid having to recompile postgres when I
wanted things not to be *that* slow (AtEOXactBuffers() I am looking at
you). But I wouldn't be very sad if it'd go.

Anybody against that?

I have used it too (for a different reason IIRC), but like you I
wouldn't have a problem if it weren't there.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, May 22, 2014 at 5:00 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Andres Freund wrote:

On 2014-05-22 16:37:35 -0400, Tom Lane wrote:

We could do that ... but I wonder if we shouldn't remove assert_enabled
altogether. What's the use case for turning it off? Not matching the
speed of a non-cassert build, because for instance MEMORY_CONTEXT_CHECKING
doesn't get turned off.

I've used it once or twice to avoid having to recompile postgres when I
wanted things not to be *that* slow (AtEOXactBuffers() I am looking at
you). But I wouldn't be very sad if it'd go.

Anybody against that?

I have used it too (for a different reason IIRC), but like you I
wouldn't have a problem if it weren't there.

I've used it, too, although not recently.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-05-23 07:20:12 -0400, Robert Haas wrote:

On Thu, May 22, 2014 at 5:00 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Andres Freund wrote:

On 2014-05-22 16:37:35 -0400, Tom Lane wrote:

We could do that ... but I wonder if we shouldn't remove assert_enabled
altogether. What's the use case for turning it off? Not matching the
speed of a non-cassert build, because for instance MEMORY_CONTEXT_CHECKING
doesn't get turned off.

I've used it once or twice to avoid having to recompile postgres when I
wanted things not to be *that* slow (AtEOXactBuffers() I am looking at
you). But I wouldn't be very sad if it'd go.

Anybody against that?

I have used it too (for a different reason IIRC), but like you I
wouldn't have a problem if it weren't there.

I've used it, too, although not recently.

That means you're for a (differently named) disable macro? Or is it not
recent enough that you don't care?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, May 23, 2014 at 8:15 AM, Andres Freund <andres@2ndquadrant.com> wrote:

I've used it once or twice to avoid having to recompile postgres when I
wanted things not to be *that* slow (AtEOXactBuffers() I am looking at
you). But I wouldn't be very sad if it'd go.

Anybody against that?

I have used it too (for a different reason IIRC), but like you I
wouldn't have a problem if it weren't there.

I've used it, too, although not recently.

That means you're for a (differently named) disable macro? Or is it not
recent enough that you don't care?

I'm leaning toward thinking we should just rip it out. The fact that
3 out of the 4 people commenting on this thread have used it at some
point provides some evidence that it has more than no value - but on
the other hand, there's a cost to keeping it around. The need to
guard some sections of code by both #ifdef USE_ASSERT_CHECKING and if
(assert_enabled) has occasionally been a source of confusion over the
years, and once I had a customer who I was afraid had gotten an
assert-enabled build into production and the only way to distinguish
between an assert-enabled build with assertions shut off via the GUC
and a build that didn't support them in the first place was to try
turning the GUC on and see whether it worked, which led to some
confusion. Now it looks like we need to add another macro to make
this dance even more complicated ... and I'm starting to think it's
just not worth it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, May 23, 2014 at 8:15 AM, Andres Freund <andres@2ndquadrant.com> wrote:

That means you're for a (differently named) disable macro? Or is it not
recent enough that you don't care?

I'm leaning toward thinking we should just rip it out. The fact that
3 out of the 4 people commenting on this thread have used it at some
point provides some evidence that it has more than no value - but on
the other hand, there's a cost to keeping it around.

Yeah. For the record, I've used it too (don't recall what for exactly).
But I don't think it's worth adding yet another layer of complication for.

The main argument for it given in this thread is recompile cost ...
but TBH, I have one word for anybody who's worried about that, and
that word is "ccache". If you don't have that tool installed, you're
missing out on a huge timesaver.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-05-23 09:56:03 -0400, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, May 23, 2014 at 8:15 AM, Andres Freund <andres@2ndquadrant.com> wrote:

That means you're for a (differently named) disable macro? Or is it not
recent enough that you don't care?

I'm leaning toward thinking we should just rip it out. The fact that
3 out of the 4 people commenting on this thread have used it at some
point provides some evidence that it has more than no value - but on
the other hand, there's a cost to keeping it around.

Yeah. For the record, I've used it too (don't recall what for exactly).
But I don't think it's worth adding yet another layer of complication for.

Cool. Seems like we have an agreement then.

The next question is whether to wait till after the branching with this?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@2ndquadrant.com> writes:

The next question is whether to wait till after the branching with this?

+1 for waiting (it's only a couple weeks anyway). This isn't a
user-facing feature in any way, so I feel no urgency to ship it in 9.4.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-05-23 10:01:37 -0400, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

The next question is whether to wait till after the branching with this?

+1 for waiting (it's only a couple weeks anyway). This isn't a
user-facing feature in any way, so I feel no urgency to ship it in 9.4.

Here's my patch for this that I plan to apply later.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-06-19 19:31:28 +0200, Andres Freund wrote:

On 2014-05-23 10:01:37 -0400, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

The next question is whether to wait till after the branching with this?

+1 for waiting (it's only a couple weeks anyway). This isn't a
user-facing feature in any way, so I feel no urgency to ship it in 9.4.

Here's my patch for this that I plan to apply later.

Hm, that missed a couple things. Updated patch attached. Besides adding
the missed documentation adjustments this also removes the -A
commandline/startup packet parameter since it doesn't serve a purpose
anymore.
Does anyone think we should rather keep -A and have it not to anything?
It seems fairly unlikely, but not impossible, that someone had the
great idea to add -A off in their connection options.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres@2ndquadrant.com> writes:

Hm, that missed a couple things. Updated patch attached. Besides adding
the missed documentation adjustments this also removes the -A
commandline/startup packet parameter since it doesn't serve a purpose
anymore.
Does anyone think we should rather keep -A and have it not to anything?
It seems fairly unlikely, but not impossible, that someone had the
great idea to add -A off in their connection options.

I think we could delete it. If anyone is using that, I think they'd
be happier getting an error than having it silently not do what they
want (and more slowly than before ...)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers