Opened 12 years ago
Closed 12 years ago
Last modified 12 years ago
#940 closed defect (fixed)
Can't compile PostGIS against PostgreSQL 9.1 beta1
|Reported by:||robe||Owned by:||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: *** [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.
Change History (21)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
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.
comment:3 by , 12 years ago
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)
comment:4 by , 12 years ago
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.
comment:5 by , 12 years ago
@mcayland http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=d64713df7e5996ab3ab337b5e0901cf2c53773f9 I think that might be the one.
comment:6 by , 12 years ago
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.
comment:7 by , 12 years ago
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.
comment:8 by , 12 years ago
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!
comment:9 by , 12 years ago
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.
comment:10 by , 12 years ago
The patch can be:
#if POSTGIS_PGSQL_VERSION > 90
InitFunctionCallInfoData(fcinfo, NULL, 1, InvalidOid, NULL, NULL);
InitFunctionCallInfoData(fcinfo, NULL, 1, NULL, NULL);
comment:11 by , 12 years ago
|Status:||new → assigned|
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.
comment:12 by , 12 years ago
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.
comment:13 by , 12 years ago
Why care about alpha when beta is out?
comment:14 by , 12 years ago
agreed, we dont' want to support any alpha, if beta changes interfaces already, and is out.
comment:15 by , 12 years ago
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.
comment:16 by , 12 years ago
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.
comment:17 by , 12 years ago
|Status:||assigned → closed|
fixed at r7135
comment:18 by , 12 years ago
Apart from the use of spaces rather than tabs for block indentation, this looks fine.
comment:19 by , 12 years ago
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?
comment:20 by , 12 years ago
Fixed in 1.5 branch also at r7197
comment:21 by , 12 years ago
Fixed in 1.4 branch also at r7203
Yap looks like it changed. InitFunctionCallInfoData in prior versions took 5 args
In 9.1b1 it looks like
Looks like they added Collation.