tds-fdw / tds_fdw

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

1.0.8 error while building on PostgreSQL 12rc1 #211

Closed jcarnu closed 4 years ago

jcarnu commented 5 years ago

Hello, I encounter the same issue than @devrimgunduz in #181, but on PostgreSQL 12rc1. I was unable to build. Could you take a look ? I think the root cause is the same (or it looks like).

Context :

CentOS 7 with the following packages installed :

freetds-libs-1.1.11-1.el7.x86_64
freetds-1.1.11-1.el7.x86_64
freetds-devel-1.1.11-1.el7.x86_64
llvm-toolset-7-llvm-libs-5.0.1-8.el7.x86_64
llvm-toolset-7-5.0.1-4.el7.x86_64
llvm5.0-libs-5.0.1-7.el7.x86_64
llvm5.0-5.0.1-7.el7.x86_64
llvm-toolset-7-runtime-5.0.1-4.el7.x86_64
llvm-toolset-7-llvm-5.0.1-8.el7.x86_64
llvm-toolset-7-libomp-5.0.1-2.el7.x86_64
llvm-toolset-7-python2-lit-0.5.1-1.el7.noarch
llvm-toolset-7-clang-libs-5.0.1-4.el7.x86_64
llvm-toolset-7-lldb-5.0.1-4.el7.x86_64
llvm5.0-devel-5.0.1-7.el7.x86_64
postgresql12-llvmjit-12rc1-1PGDG.rhel7.x86_64
llvm-toolset-7-compiler-rt-5.0.1-2.el7.x86_64
llvm-toolset-7-clang-5.0.1-4.el7.x86_64

I use a curl based download of v1.0.8 (I also tried 1.0.7 with no better result).

Compile output

make USE_PGXS=1
gcc -std=gnu99 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -fPIC -I./include/ -I. -I./ -I/usr/pgsql-12/include/server -I/usr/pgsql-12/include/internal  -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 ‘tdsGetColumnMetadata’:
src/tds_fdw.c:1255:70: error: invalid type argument of ‘->’ (have ‘FormData_pg_attribute’)
     char* local_name = festate->attinmeta->tupdesc->attrs[local_ncol]->attname.data;
                                                                      ^
src/tds_fdw.c:1260:71: error: invalid type argument of ‘->’ (have ‘FormData_pg_attribute’)
      column->attr_oid = festate->attinmeta->tupdesc->attrs[local_ncol]->atttypid;
                                                                       ^
In file included from /usr/pgsql-12/include/server/postgres.h:47:0,
                 from src/tds_fdw.c:28:
src/tds_fdw.c:1293:47: error: invalid type argument of ‘->’ (have ‘FormData_pg_attribute’)
       festate->attinmeta->tupdesc->attrs[ncol]->attname.data)
                                               ^
/usr/pgsql-12/include/server/utils/elog.h:125:14: note: in definition of macro ‘ereport_domain’
    errfinish rest; \
              ^
src/tds_fdw.c:1289:5: note: in expansion of macro ‘ereport’
     ereport(WARNING,
     ^
src/tds_fdw.c: In function ‘tdsIterateForeignScan’:
src/tds_fdw.c:1643:5: warning: implicit declaration of function ‘ExecStoreTuple’ [-Wimplicit-function-declaration]
     ExecStoreTuple(tuple, slot, InvalidBuffer, false);
     ^
make: *** [src/tds_fdw.o] Error 1

Feel free to request more details .

juliogonzalez commented 5 years ago

First of all: thanks for an excellent bug report. We don't get such good reports frequently :-)

Not being the main developer, but only the guy that maintains the CI and tries to help, my advice is: try 2.0.0-alpha.3

I am pretty sure that if I try our CI with 1.0.8 and PostgreSQL 12 (still not added to the CI for master), it will fail as it did for the other bug you linked.

At some point there should be tds_fdw 2.0.0, but that requires C developers joining the project and helping @GeoffMontee (BTW, hacktoberfest could be a good opportunity).

I just proposed #212, but it could be very well be that we are NOT ready yet.

jcarnu commented 5 years ago

Thanks for your feedback, I'm trying to give as much information as I would have on my own project when an issue is found. So happy it helps.

On my side I tried to better define the problem :

In this commit : https://git.postgresql.org/gitweb/?p=postgresql.git;a=blobdiff;f=src/include/nodes/primnodes.h;h=860a84de7c00838ce20159073303907bc35b1c2f;hp=86ec82eaaae8637918b3d1a1df84ed20978b8192;hb=HEAD;hpb=39151781c8cd2c8bf6057496426fb9c07178eda5

ArrayRef is replaced by SubscriptingRef.

I tried to bulk replace s/ArrayRef/SubscriptingRef/g in deparse.c and HEAD compiles on a debian buster system. I'm going to test it against a centos/7 asap.

This might be tricky to overcome the situation, as ArrayRef was renamed, we should, to be backward compatible, have two versions of the deparse.c file. If my assumptions are correct and tds is working (I've not tested it yet as I told you).

I'll be back in few hours after building it and testing it on my vagrant machine.

If thing are right I would propose a patch (not a MR) to give a way to solve this in 12rc1 (maybe in 11). Maybe @GeoffMontee would find a more elegant way to solve that renaming problem ?

Thanks again for your feedback.

jcarnu commented 5 years ago

Well, I tried to run CREATE EXTENSION tds_fdw;

There's a problem with ExecTupleStore() with 12RC1.

This commit : https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=29c94e03c7d05d2b29afa1de32795ce178531246

explains that ExecTupleStore was split into two functions. This should be taken into account into that new version support.

It's late here, I have to stop my investigations for now. Hope this helps.