Can't compile PostGIS against PostgreSQL 9.1 beta1

[robe]

I think something has changed in maybe the aggregation logic of 9.1. I was able to compile postgis against 9.1 alpha5 (saw I thought anyway). Just switched to 9.1 beta1 and now I get this error.

 -Ic:/projects/pg/pg91b1/include/POSTGR~1/server/port/win32  -c -o lwgeom_accum.o lwgeom_accum.c
lwgeom_accum.c:306:54: macro "InitFunctionCallInfoData" requires 6 arguments, but only 5 given
lwgeom_accum.c: In function `PGISDirectFunctionCall1':
lwgeom_accum.c:306: error: `InitFunctionCallInfoData' undeclared (first use in this function)
lwgeom_accum.c:306: error: (Each undeclared identifier is reported only once
lwgeom_accum.c:306: error: for each function it appears in.)
make[1]: *** [lwgeom_accum.o] Error 1

Can someone else confirm who has PostgreSQL 9.1 beta 1 installed confirm this is a real issue and not limited to my hacked MingW setup.

[robe]

Yap looks like it changed. InitFunctionCallInfoData? in prior versions took 5 args

InitFunctionCallInfoData 	( 	Fcinfo,
		Flinfo,
		Nargs,
		Context,
		Resultinfo  		 )  

In 9.1b1 it looks like

InitFunctionCallInfoData(Fcinfo, Flinfo, Nargs, Collation, Context, Resultinfo)

Looks like they added Collation.

[otto]

Not sure if this is a proper fix but I changed:

-   InitFunctionCallInfoData(fcinfo, NULL, 1, NULL, NULL);
+   InitFunctionCallInfoData(fcinfo, NULL, 1, InvalidOid, NULL, NULL);

and it appears to work ok for me.

[strk]

A proper fix supports both old and new versions of postgresql. Using a macro is likely the way to go. Best if provided by pgsq headers already (back to 8.4)

[mcayland]

Please can you post a reference to the gitweb entry on git.postgresql.org for this chance so we can verify that this is, in fact, the right thing to do?

I suspect it is related to the recent collation changes.

[otto]

@mcayland  http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=d64713df7e5996ab3ab337b5e0901cf2c53773f9 I think that might be the one.

[mcayland]

Yeah that looks like it. So the next question is: what are we comparing? If it's geometries then InvalidOid? is probably correct because collation is irrelevant on binary types like in this case.

[robe]

We are comparing geometries. Our code in lwgeom_accum doesn't deal with anything else but geometries. though I wonder if we use that function elsewhere. Anyrate most of the code in our code base only works with spatial types and when it doesn't they are pre-defined string modifiers.

[robe]

Guys,

Any news on this. I really want/need to start stress testing PostgreSQL 9.1 beta 1 with PostGIS 2.0.

On the bright side, because of this collation change, I guess that means PostgreSQL 9.1 won't work with PostGIS 1.5 anymore. Which means everyone running PostgreSQL 9.1 that wants to use PostGIS will have to run PostGIS 2.0. So they'll have to wait for us even if they beat us to the finish line -- yee pee!

[mcayland]

In that case, the InvalidOid? fix looks good to me. You'll need to put it inside a PGSQL_MAJOR_VERSION/PGSQL_MINOR_VERSION #ifdef to make sure it doesn't break older versions.

If you'd like to propose a patch, I'd be happy to take a look.

[stl]

The patch can be:

#if POSTGIS_PGSQL_VERSION > 90

InitFunctionCallInfoData?(fcinfo, NULL, 1, InvalidOid?, NULL, NULL);

#else

InitFunctionCallInfoData?(fcinfo, NULL, 1, NULL, NULL);

#endif

[robe]

Mark, stl's patch works for me and all my mingw 8.4,9.0, 9.1 tested against vc++ builds seem to pass regress (except the raster regress which I'll inspect in a bit to make sure they are real failures and not just line break issues and so forth). If you are okay with this change, I'd like to commit my patched lwgeom_accum.c

I'd like to release a new set of windows experimental builds with geos-3.3.0-rc1 and need this in before I can do that.

Thanks, Regina

[stl]

The patch will fail for all 9.1 alfa versions of PostgreSQL. POSTGIS_PGSQL_VERSION parameter is the same for alfa and beta versions. We need a better parameter which can differentiate between alfa and beta versions of PostgreSQL.

[nicklas]

Why care about alpha when beta is out?

[strk]

agreed, we dont' want to support any alpha, if beta changes interfaces already, and is out.

[robe]

Yah the EnterpriseDb? already has their beta 1 build out which is what I'm using to test against. So I try to keep the PostGIS experimental builds in line with theirs since most Windows users will get it from them. I think the same is true with yum managed by Devrim -- but that is next on my todo to compile on my CentOS box testing with his 9.1beta yum install and then compile PostGIS 2.0 on that.

So really there isn't a reason for anyone to be using an alpha build.

[robe]

Does anyone have issues with me committing this patch? Speak now?

I figure since Mark seems a bit disposed, he can take it out out later or modify it if he has issue, but I really got to move on with packaging and testing the ports of my projects and this is holding me up.

[robe]

fixed at r7135

[mcayland]

Apart from the use of spaces rather than tabs for block indentation, this looks fine.

[robe]

okay I applied our astyle script to it. Should be all set. We seem to have a lot of files not astyled right again or is it my version of astyle 1.23?

[pramsey]

Fixed in 1.5 branch also at r7197

[pramsey]

Fixed in 1.4 branch also at r7203