pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Missing errcode() in ereport
Date: Fri, 20 Mar 2020 10:40:21 -0400
Message-ID: <15922.1584715221@sss.pgh.pa.us> (raw)
In-Reply-To: <20200320045637.n3xo6gcmbqcg42tq@alap3.anarazel.de>
References: <CAOBaU_bb7FR5cF9RKN42kPHf6pk9CeQ=wTuieJv-t3MroU_q0g@mail.gmail.com>
	<20200317095839.GG2206@paquier.xyz>
	<CAA4eK1+powcFw2EAoVgvH4P3fspAJhxU-VTy0_cR2XpcBZGMAA@mail.gmail.com>
	<32174.1584454158@sss.pgh.pa.us>
	<20200319175902.vjib2tslosw65iyr@alap3.anarazel.de>
	<5063.1584641224@sss.pgh.pa.us>
	<20200319230446.lxjybufdcgwaqkhj@alap3.anarazel.de>
	<5995.1584660775@sss.pgh.pa.us>
	<20200320015908.ajxneh4jx6avqegv@alap3.anarazel.de>
	<9966.1584671550@sss.pgh.pa.us>
	<20200320045637.n3xo6gcmbqcg42tq@alap3.anarazel.de>

Andres Freund <andres@anarazel.de> writes:
> On 2020-03-19 22:32:30 -0400, Tom Lane wrote:
>> Could we get away with moving the compiler goalposts for the back
>> branches?  I dunno, but it's a fact that we aren't testing anymore
>> with any compilers that would complain about unconditional use of
>> __VA_ARGS__.  So it might be broken already and we wouldn't know it.

> FWIW, I did grep for unprotected uses, and  didn't find anything.

Yeah, I also grepped the v11 branch for that.

>> (I suspect the last buildfarm animal that would've complained about
>> this was pademelon, which I retired more than a year ago IIRC.)

> I guess a query that searches the logs backwards for animals without
> __VA_ARGS__ would be a good idea?

I did a more wide-ranging scan, looking at the 9.4 branch as far back
as 2015.  Indeed, pademelon is the only animal that ever reported
not having __VA_ARGS__ in that timeframe.

So I've got mixed emotions about this.  On the one hand, it seems
quite unlikely that anyone would ever object if we started requiring
__VA_ARGS__ in the back branches.  On the other hand, it's definitely
not project policy to change requirements like that in minor releases.
Also the actual benefit might not be much.  If anyone did mistakenly
back-patch a fix that included a paren-free ereport, the buildfarm
would point out the error soon enough.

I thought for a little bit about making the back branches define ereport
with "..." if HAVE__VA_ARGS and otherwise not, but if we did that then
the buildfarm would *not* complain about paren-free ereports in the back
branches.  I think that would inevitably lead to there soon being some,
so that we'd effectively be requiring __VA_ARGS__ anyway.  (I suppose
I could resurrect pademelon ... but who knows whether that old war
horse will keep working for another four years till v11 is retired.)

On balance I'm leaning towards keeping the parens as preferred style
for now, adjusting v12 so that the macro will allow paren omission
but we don't break ABI, and not touching the older branches.  But
if there's a consensus to require __VA_ARGS__ in the back branches,
I won't stand in the way.

			regards, tom lane





view thread (31+ messages)  latest in thread

Message-ID: <15922.1584715221@sss.pgh.pa.us>
Permalink:  ../15922.1584715221@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/15922.1584715221@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, amit.kapila16@gmail.com, michael@paquier.xyz, rjuju123@gmail.com, masahiko.sawada@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: Missing errcode() in ereport
  In-Reply-To: <15922.1584715221@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox