On 10/15/19 5:23 AM, Andrew Gierth wrote:
>>>>>> "Mark" == Mark Dilger <hornschnorter@gmail.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).
I don't really dispute your claim, but it doesn't unambiguously follow
from the wording of the comment. The part that tripped me up while
reviewing Peter's patch is that he changed the preprocessor logic to use
#ifdef rather than #if, implying that he believes HSTORE_IS_HSTORE_NEW
will only be defined when true, and undefined when false, rather than
something like:
#if OLD_STUFF
#define HSTORE_IS_HSTORE_NEW 0
#else
#define HSTORE_IS_HSTORE_NEW 1
#endif
which is admittedly a less common coding pattern than only defining it
when true, but the choice of #if rather than #ifdef in the original
sources might have been intentional.
I tried briefly to download this project from pgfoundry without success.
Do you have a copy of the relevant code where you can see how this
gets defined, and can you include it in a reply?
Thanks,
mark