Re: Fix most -Wundef warnings

From: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
To: Mark Dilger <hornschnorter(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Fix most -Wundef warnings
Date: 2019-10-15 12:23:43
Message-ID: 875zkqz3ao.fsf@news-spur.riddles.org.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>>>> "Mark" == Mark Dilger <hornschnorter(at)gmail(dot)com> writes:

>> +#ifdef HSTORE_IS_HSTORE_NEW

Mark> Checking the current sources, git history, and various older
Mark> commits, I did not find where HSTORE_IS_HSTORE_NEW was ever
Mark> defined.

In contrib/hstore, it never was.

The current version of contrib/hstore had a brief life as a separate
extension module called hstore-new, which existed to backport its
functionality into 8.4. The data format for hstore-new was almost
identical to the new contrib/hstore one (and thus different from the old
contrib/hstore), and changed at one point before its final release, so
there were four possible upgrade paths as explained in the comments.

The block comment with the most pertinent explanation seems to have
been a victim of pgindent, but the relevant part is this:

* [...] So the upshot of all this
* is that we can treat all the edge cases as "new" if we're being built
* as hstore-new, and "old" if we're being built as contrib/hstore.

So, HSTORE_IS_HSTORE_NEW was defined if you were building a pgxs module
called "hstore-new" (which was distributed separately on pgfoundry but
the C code was the same), and not if you're building "hstore" (whether
an in or out of tree build).

Mark> The check on HSTORE_IS_HSTORE_NEW goes back at least as far as
Mark> 2006, suggesting it was needed for migrating from some version
Mark> pre-9.0, making me wonder if anybody would need this in the
Mark> field. Should we drop support for this? I don't have a strong
Mark> reason to advocate dropping support other than that this #define
Mark> appears to be undocumented.

The only reason not to remove most of hstore_compat.c is that there is
no way to know what data survives in the wild in each of the three
possible hstore formats (8.4 contrib, pre-final hstore-new, current). I
think it's most unlikely that any of the pre-final hstore-new data still
exists, but how would anyone know?

(The fact that there have been exactly zero field reports of either of
the WARNING messages unfortunately doesn't prove much. Almost all
possible non-current hstore values are unambiguously in one or other of
the possible formats, the ambiguity is only possible because the old
code didn't always set the varlena length to the correct size, but left
unused space at the end.)

--
Andrew (irc:RhodiumToad)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message PG Bug reporting form 2019-10-15 13:11:29 BUG #16059: Tab-completion of filenames in COPY commands removes required quotes
Previous Message Peter Geoghegan 2019-10-15 12:05:32 Re: tuplesort test coverage