tds_fdw 1.0.4 broken for postgres 9.3?

[juliogonzalez]

Hi.

I'm updating the repo I have to create RPMs, and noticed I can compile tds_fdw for PostgreSQL 9.4, but not for PostgreSQL 9.3.

Building from the tar.gz file (following CentOS instructions) is showing the same error for CentOS6

[root@centos6-test tds_fdw-1.0.4]# PATH=/usr/pgsql-9.3/bin:$PATH make USE_PGXS=1
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -I/usr/include/et -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I./include/ -I. -I. -I/usr/pgsql-9.3/include/server -I/usr/pgsql-9.3/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 'tdsConvertToCString':
src/tds_fdw.c:1023: error: 'make_timestamp' undeclared (first use in this function)
src/tds_fdw.c:1023: error: (Each undeclared identifier is reported only once
src/tds_fdw.c:1023: error: for each function it appears in.)
make: *** [src/tds_fdw.o] Error 1

And CentOS7:

[root@centos7-test tds_fdw-1.0.4]# PATH=/usr/pgsql-9.3/bin:$PATH make USE_PGXS=1
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -DLINUX_OOM_ADJ=0 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -fpic -I./include/ -I. -I. -I/usr/pgsql-9.3/include/server -I/usr/pgsql-9.3/include/internal -D_GNU_SOURCE -I/usr/include/libxml2  -I/usr/include  -c -o src/tds_fdw.o src/tds_fdw.c
In file included from /usr/pgsql-9.3/include/server/funcapi.h:19:0,
                 from src/tds_fdw.c:29:
src/tds_fdw.c: In function ‘tdsConvertToCString’:
src/tds_fdw.c:1023:41: error: ‘make_timestamp’ undeclared (first use in this function)
      datetime_out = DirectFunctionCall6(make_timestamp, 
                                         ^
/usr/pgsql-9.3/include/server/fmgr.h:554:26: note: in definition of macro ‘DirectFunctionCall6’
  DirectFunctionCall6Coll(func, InvalidOid, arg1, arg2, arg3, arg4, arg5, arg6)
                          ^
src/tds_fdw.c:1023:41: note: each undeclared identifier is reported only once for each function it appears in
      datetime_out = DirectFunctionCall6(make_timestamp, 
                                         ^
/usr/pgsql-9.3/include/server/fmgr.h:554:26: note: in definition of macro ‘DirectFunctionCall6’
  DirectFunctionCall6Coll(func, InvalidOid, arg1, arg2, arg3, arg4, arg5, arg6)
                          ^
make: *** [src/tds_fdw.o] Error 1

Building the extension for PostgreSQL 9.4 doesn't show any problem.

Building 1.0.3 (previous version) for PostgreSQL 9.3 doesn't show any problem.

In all cases the PostgreSQL packages are the official from yum.postgresql.org

[GeoffMontee]

@juliogonzalez,

Ah, sorry about that! I didn't realize that make_timestamp() was only recently added in PostgreSQL 9.4. I've committed a fix for this:

https://github.com/GeoffMontee/tds_fdw/commit/bdab7612a2424a131a895e816c920bf39f3b471d

I also created a version 1.0.5 release with the fix:

https://github.com/GeoffMontee/tds_fdw/releases/tag/v1.0.5

Can you please see if you have any issues building that version on PostgreSQL 9.3?

[juliogonzalez]

No problems building 1.0.5, and the extension is loading without any further errors:

-bash-4.1$ psql -d test
psql (9.4.4, server 9.3.9)
Type "help" for help.

test=# \dx
                                            List of installed extensions
  Name   | Version |   Schema   |                                    Description                                    
---------+---------+------------+-----------------------------------------------------------------------------------
 plpgsql | 1.0     | pg_catalog | PL/pgSQL procedural language
 tds_fdw | 1.0.5   | public     | Foreign data wrapper for querying a TDS database (Sybase or Microsoft SQL Server)
(2 rows)

However there's a warning which only happens for 9.3 but not for 9.4 (warning was not present at 1.0.3)

gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -DLINUX_OOM_ADJ=0 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -fpic -I./include/ -I. -I. -I/usr/pgsql-9.3/include/server -I/usr/pgsql-9.3/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 'tdsConvertToCString':
src/tds_fdw.c:981:10: warning: unused variable 'erc' [-Wunused-variable]
  RETCODE erc;
          ^
src/tds_fdw.c:980:12: warning: unused variable 'datetime_in' [-Wunused-variable]
  DBDATEREC datetime_in;
            ^

[juliogonzalez]

Not critical of course, just for you to know ;-)

[GeoffMontee]

@juliogonzalez,

Thanks for testing that out!

I've committed a fix to make the warning go away:

https://github.com/GeoffMontee/tds_fdw/commit/727fae3d25b9842d674bdc2be971cedd20a35c3c

That fix will go into the next release after 1.0.5.

For future releases, I will have to remember to test on all supported versions of PostgreSQL. Thanks for reporting this issue!

[juliogonzalez]

Well, thanks to you for making the extension!

The commit fixes one of the two warnings. One is still present:

gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -DLINUX_OOM_ADJ=0 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -fpic -I./include/ -I. -I. -I/usr/pgsql-9.3/include/server -I/usr/pgsql-9.3/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 ‘tdsConvertToCString’:
src/tds_fdw.c:980:12: warning: unused variable ‘datetime_in’ [-Wunused-variable]
  DBDATEREC datetime_in;
            ^

I'll try to set up a small continous integration for this, so 9.3, 9.4 and 9.5 builds are tested automatically . Will be useful for my RPM, but if you're interested on adding it to your repo, let me know and I'll send you notice when it's ready.

[GeoffMontee]

@juliogonzalez,

The commit fixes one of the two warnings. One is still present:

Whoops, sorry about that. I've fixed the code, so that the other declaration happens inside the #if as well.

https://github.com/GeoffMontee/tds_fdw/commit/683b7244597f1cabcde807dd32a69ee38195c67f

This function is starting to look a bit messy. At some point, I should probably move this datetime conversion code code into a separate function, and then call that from tdsConvertToCString().

I'll try to set up a small continous integration for this, so 9.3, 9.4 and 9.5 builds are tested automatically . Will be useful for my RPM, but if you're interested on adding it to your repo, let me know and I'll send you notice when it's ready.

Sure, that sounds great. Thanks!

[juliogonzalez]

Yes, the commit fixes the last warning, so you can close the issue.

Thanks!

[GeoffMontee]

Build failure was fixed in 1.0.5, and compiler warnings were fixed in 1.0.6. Closing this.

Thanks again for reporting this!