Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#940 closed defect (fixed)

Can't compile PostGIS against PostgreSQL 9.1 beta1

Reported by: robe Owned by: robe
Priority: blocker Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: Cc:

Description

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.

Change History (21)

comment:1 by robe, 13 years ago

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.

comment:2 by otto, 13 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 strk, 13 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 mcayland, 13 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:6 by mcayland, 13 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 robe, 13 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 robe, 13 years ago

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!

comment:9 by mcayland, 13 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 stl, 13 years ago

The patch can be:

#if POSTGIS_PGSQL_VERSION > 90

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

#else

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

#endif

comment:11 by robe, 13 years ago

Owner: changed from pramsey to robe
Status: newassigned

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

comment:12 by stl, 13 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 nicklas, 13 years ago

Why care about alpha when beta is out?

comment:14 by strk, 13 years ago

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

comment:15 by robe, 13 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 robe, 13 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 robe, 13 years ago

Resolution: fixed
Status: assignedclosed

fixed at r7135

comment:18 by mcayland, 13 years ago

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

comment:19 by robe, 13 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 pramsey, 13 years ago

Fixed in 1.5 branch also at r7197

comment:21 by pramsey, 13 years ago

Fixed in 1.4 branch also at r7203

Note: See TracTickets for help on using tickets.