Opened 7 weeks ago

Closed 5 weeks ago

#3867 closed defect (fixed)

Can't load against PostgreSQL 11 head DatumGetJsonb renamed

Reported by: robe Owned by: pramsey
Priority: blocker Milestone: PostGIS 2.4.1
Component: postgis Version: trunk
Keywords: Cc:


psql:/var/lib/jenkins/workspace/postgis/branches/2.5/regress/00-regress-install/share/contrib/postgis/postgis.sql:97: ERROR:  could not load library "/var/lib/jenkins/workspace/postgis/branches/2.5/regress/00-regress-install/lib/": /var/lib/jenkins/workspace/postgis/branches/2.5/regress/00-regress-install/lib/ undefined symbol: DatumGetJsonb

Change History (7)

comment:1 Changed 7 weeks ago by robe

I think it was this commit that broke 'r code.;a=commitdiff;h=4bd1994650fddf49e717e35f1930d62208845974

Make DatumGetFoo/PG_GETARG_FOO/PG_RETURN_FOO macro names more consistent.
author	Tom Lane <>	
	Mon, 18 Sep 2017 15:21:23 -0400 (15:21 -0400)
committer	Tom Lane <>	
	Mon, 18 Sep 2017 15:21:23 -0400 (15:21 -0400)
By project convention, these names should include "P" when dealing with a
pointer type; that is, if the result of a GETARG macro is of type FOO *,
it should be called PG_GETARG_FOO_P not just PG_GETARG_FOO.  Some newer
types such as JSONB and ranges had not followed the convention, and a
number of contrib modules hadn't gotten that memo either.  Rename the
offending macros to improve consistency.

In passing, fix a few places that thought PG_DETOAST_DATUM() returns
a Datum; it does not, it returns "struct varlena *".  Applying
DatumGetPointer to that happens not to cause any bad effects today,
but it's formally wrong.  Also, adjust an ltree macro that was designed
without any thought for what pgindent would do with it.

This is all cosmetic and shouldn't have any impact on generated code.

Mark Dilger, some further tweaks by me


Looks like DatumGetJsonb?, should now be called DatumGetJsonbP

index 24f4916..d639bbc 100644 (file)
--- a/src/include/utils/jsonb.h
+++ b/src/include/utils/jsonb.h
@@ -65,10 +65,10 @@ typedef enum
 #define JGIN_MAXLENGTH 125     /* max length of text part before hashing */
 /* Convenience macros */
-#define DatumGetJsonb(d)   ((Jsonb *) PG_DETOAST_DATUM(d))
-#define JsonbGetDatum(p)   PointerGetDatum(p)
-#define PG_GETARG_JSONB(x) DatumGetJsonb(PG_GETARG_DATUM(x))
+#define DatumGetJsonbP(d)  ((Jsonb *) PG_DETOAST_DATUM(d))
+#define JsonbPGetDatum(p)  PointerGetDatum(p)
+#define PG_GETARG_JSONB_P(x)   DatumGetJsonbP(PG_GETARG_DATUM(x))
 typedef struct JsonbPair JsonbPair;

comment:2 Changed 7 weeks ago by robe

Summary: Can't load against PostgreSQL 11 headCan't load against PostgreSQL 11 head DatumGetJsonb renamed

comment:3 Changed 7 weeks ago by robe

In 15850:

Can't load against PostgreSQL 11 head DatumGetJsonb? renamed
References #3867 for PostGIS 2.5

comment:4 Changed 7 weeks ago by robe

Milestone: PostGIS 2.5.0PostGIS 2.4.1

I'll backport this to PostGIS 2.4.1 after we have released 2.4.0.

comment:5 Changed 7 weeks ago by robe

I haven't backported this yet because strk said he wanted to define it as a macro for pre-11 versions and then call the macro instead of the if-def clauses everywhere we need them.

pramsey you okay with that? For 2.4.1 I want to just go with the IF-Def as done above since we aren't going to be augmenting that and I hate indirection code that is only used in one place.

We don't use ranges yet and I think JSONB is the only one at issue here. For 2.5 main benefit I see to having it as a macro is that if we do use JSONB in other areas of our code (which sounds likely) -- e.g. we create a ST_AsGeoJSON that can take an anyelement and return a fully qualified jsonb object, then having this isolated to the macro would be nice.

comment:6 Changed 7 weeks ago by pramsey

Yes, that approach (macro) is fine. I actually looked to see if pgsql had one already but no.

comment:7 Changed 5 weeks ago by pramsey

Resolution: fixed
Status: newclosed

In 15964:

PgSQL 11 support for 2.4 branch (DatumGetJsonbP macro)
Closes #3867

Note: See TracTickets for help on using tickets.