tds-fdw / tds_fdw

A PostgreSQL foreign data wrapper to connect to TDS databases (Sybase and Microsoft SQL Server)
Other
381 stars 102 forks source link

Version 1.0.6 does not compile for PostgreSQL 9.5 rc1 #49

Closed juliogonzalez closed 8 years ago

juliogonzalez commented 8 years ago
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -I/usr/include/et -fpic -I./include/ -I. -I./ -I/usr/pgsql-9.5/include/server -I/usr/pgsql-9.5/include/internal -I/usr/include/et -D_GNU_SOURCE -I/usr/include/libxml2  -I/usr/include  -c -o src/tds_fdw.o src/tds_fdw.c
src/tds_fdw.c: In function ‘tds_fdw_handler’:
src/tds_fdw.c:103: warning: assignment from incompatible pointer type
src/tds_fdw.c: In function ‘tdsGetForeignPaths’:
src/tds_fdw.c:1901: warning: passing argument 8 of ‘create_foreignscan_path’ from incompatible pointer type
/usr/pgsql-9.5/include/server/optimizer/pathnode.h:82: note: expected ‘struct Path *’ but argument is of type ‘struct List *’
src/tds_fdw.c:1901: error: too few arguments to function ‘create_foreignscan_path’
src/tds_fdw.c: In function ‘tdsGetForeignPlan’:
src/tds_fdw.c:1946: error: too few arguments to function ‘make_foreignscan’
make: *** [src/tds_fdw.o] Error 1

Broken for both Ubuntu 12.04 and CentOS 6.7 with PostgreSQL official packages (http://apt.postgresql.org and http://yum.postgresql.org/). See https://jenkins-juliogonzalez.rhcloud.com/job/tds_fdw-build-ng/118/ for reference.

It seems this is due to a change happened between PostgreSQL 9.5 alpha2 (installed at the current CI and building) and 9.5 rc1 (installed at the ng CI).

GeoffMontee commented 8 years ago

Thanks for catching this bug!

It looks like create_foreignscan_path and make_foreignscan were changed in this commit.

The create_foreignscan_path change was:

 extern ForeignPath *create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
                        double rows, Cost startup_cost, Cost total_cost,
                        List *pathkeys,
                        Relids required_outer,
 +                      Path *fdw_outerpath,
                        List *fdw_private);

I think that I should just be able to set fdw_outerpath to NULL here until tds_fdw supports join pushdowns.

The make_foreignscan change was:

 extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
                 Index scanrelid, List *fdw_exprs, List *fdw_private,
 -               List *fdw_scan_tlist, List *fdw_recheck_quals);
 +               List *fdw_scan_tlist, List *fdw_recheck_quals,
 +               Plan *outer_plan);

For this, I think that I should be able to set outer_plan to NULL here.

GeoffMontee commented 8 years ago

GetForeignPlan also changed in the commit referenced above:

 typedef ForeignScan *(*GetForeignPlan_function) (PlannerInfo *root,
                                                         RelOptInfo *baserel,
                                                          Oid foreigntableid,
                                                      ForeignPath *best_path,
                                                             List *tlist,
 -                                                       List *scan_clauses);
 +                                                        List *scan_clauses,
 +                                                         Plan *outer_plan

So I need to change the definition of tdsGetForeignPlan here and here.

GeoffMontee commented 8 years ago

I pushed a commit, but I forgot to wrap some of the changes in #ifdefs, so I need to fix it: https://github.com/GeoffMontee/tds_fdw/commit/080980020c9f949e9028db1591633bdf031a4e10

GeoffMontee commented 8 years ago

The latest commit fixes the issue in 9.5 for me, and still compiles on 9.4: https://github.com/GeoffMontee/tds_fdw/commit/cd84f7bee384b830092ce087a016b2d6ff0c3bb3

juliogonzalez commented 8 years ago

Yes, cd84f7b compiles and passes all acceptance tests for all versions (9.1 to 9.5): https://jenkins-juliogonzalez.rhcloud.com/job/tds_fdw-build/35/

I think the issue can be closed. Great work!

GeoffMontee commented 8 years ago

Awesome! It's really cool to see the automated tests run on Jenkins.

This fix will be in 1.0.7.

devrimgunduz commented 8 years ago

When will 1.0.7 be released? 9.5.0 is out tomorrow, so...

GeoffMontee commented 8 years ago

Hi @devrimgunduz,

I actually just changed the version strings to 1.0.7. The release should happen tonight.

devrimgunduz commented 8 years ago

Thanks! I'll keep my eye on it.

GeoffMontee commented 8 years ago

@devrimgunduz,

Here it is: https://github.com/GeoffMontee/tds_fdw/releases/tag/v1.0.7