nkhorman / json_fdw

PostgreSQL extension which implements a Foreign Data Wrapper (FDW) for JSON files.
109 stars 12 forks source link

Error Building on OS X 10.8 #1

Closed theory closed 10 years ago

theory commented 10 years ago

Just tried to build on OS X Mountain Lion 10.8.5 with cmake 2.8.12 and yajl 2.0.1 but got some errors:

> make
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv  -I. -I. -I/usr/local/pgsql/include/server -I/usr/local/pgsql/include/internal -D_XOPEN_SOURCE -I/usr/local/include/libxml2  -I/usr/local/include  -c -o json_fdw.o json_fdw.c
json_fdw.c:656:52: error: invalid application of 'sizeof' to an incomplete type
      'HeapTupleHeaderData' (aka 'struct HeapTupleHeaderData')
  ...tupleWidth = MAXALIGN(baserel->width) + MAXALIGN(sizeof(HeapTupleHeaderData));
                                                      ^     ~~~~~~~~~~~~~~~~~~~~~
/usr/local/pgsql/include/server/c.h:540:53: note: expanded from macro 'MAXALIGN'
#define MAXALIGN(LEN)                   TYPEALIGN(MAXIMUM_ALIGNOF, (LEN))
                                                                    ^
/usr/local/pgsql/include/server/c.h:534:16: note: expanded from macro 'TYPEALIGN'
        (((intptr_t) (LEN) + ((ALIGNVAL) - 1)) & ~((intptr_t) ((ALIGNVAL) - 1)))
                      ^
/usr/local/pgsql/include/server/access/htup.h:21:16: note: forward declaration of
      'struct HeapTupleHeaderData'
typedef struct HeapTupleHeaderData HeapTupleHeaderData;
               ^
json_fdw.c:1479:35: warning: implicit declaration of function 'heap_form_tuple' is invalid
      in C99 [-Wimplicit-function-declaration]
                        sampleRows[sampleRowCount++] = heap_form_tuple(tupleDescriptor, 
                                                       ^
json_fdw.c:1479:33: warning: incompatible integer to pointer conversion assigning to
      'HeapTuple' (aka 'struct HeapTupleData *') from 'int' [-Wint-conversion]
                        sampleRows[sampleRowCount++] = heap_form_tuple(tupleDescriptor, 
                                                     ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
json_fdw.c:1506:5: warning: implicit declaration of function 'heap_freetuple' is invalid
      in C99 [-Wimplicit-function-declaration]
                                heap_freetuple(sampleRows[rowIndex]);
                                ^
json_fdw.c:1507:26: warning: incompatible integer to pointer conversion assigning to
      'HeapTuple' (aka 'struct HeapTupleData *') from 'int' [-Wint-conversion]
                                sampleRows[rowIndex] = heap_form_tuple(tupleDescriptor,
                                                     ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4 warnings and 1 error generated.
make: *** [json_fdw.o] Error 1
pykello commented 10 years ago

Thanks for the report. Which PostgreSQL version are you using?

theory commented 10 years ago

This is a build from master a few days ago.

theory commented 10 years ago

Sorry, no, my mistake. I am running the build from master, but this was built against v9.3.0.

ozgune commented 10 years ago

Hi David,

We wrote the json_fdw against PostgreSQL 9.2, and haven't yet had a chance to upgrade to v9.3 header files. I tried 9.3 on my box, and ran into the same problem.

We are heads down working on the next version (Citus v3.0) of our product -- our current version is based on Postgres 9.2, and the next one will be based on 9.3. We intend to upgrade json_fdw to the newer APIs as we near our v3.0 release.

In the interim, does building json_fdw against 9.2 headers work for you?

If it does, could you let us know? We manually pass in a dynamic linker flag in the Makefile -- that flag won't work on OS X, but we think we could have a simple fix on that one.

Thanks, Ozgun

theory commented 10 years ago
> make             
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv  -bundle -multiply_defined suppress -o json_fdw.so json_fdw.o -L/usr/local/pgsql-9.2/lib -L/usr/local/lib -Wl,-dead_strip_dylibs   -lz -l:libyajl.so.2 -bundle_loader /usr/local/pgsql-9.2/bin/postgres
ld: library not found for -l:libyajl.so.2
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [json_fdw.so] Error 1
> ll /usr/local/lib/libyajl*
-rwxr-xr-x  1 root  wheel    40K Oct 23 09:17 /usr/local/lib/libyajl.2.0.1.dylib*
lrwxr-xr-x  1 root  wheel    19B Oct 23 09:17 /usr/local/lib/libyajl.2.dylib@ -> libyajl.2.0.1.dylib
lrwxr-xr-x  1 root  wheel    15B Oct 23 09:17 /usr/local/lib/libyajl.dylib@ -> libyajl.2.dylib
-rw-r--r--  1 root  wheel    46K Oct 23 09:17 /usr/local/lib/libyajl_s.a
pykello commented 10 years ago

The reason for using the -l:libyajl.so.2 instead of -lyajl flag was that in Ubuntu 12.04 the yajl installed by apt-get is yajl 1.0, and even after manually installing yajl 2.0, the -lyajl links against yajl 1.0 instead of yajl 2.0. But the "-l:" syntax doesn't work in some non-Linux Unix's (e.g. FreeBSD and OS X).

To solve this issue, I tried changing the "SHLIB_LINK = -lz -l:libyajl.so.2" line in the Makefile to the following:

ifeq ($(shell uname -s), Linux)   # Directly link against yajl 2, so it works in Ubuntu 12.04 too.   SHLIB_LINK = -lz -l:libyajl.so.2 else   # Non-linux OS's (in particular, OS X) don't support "-l:" syntax,   # so use the -lyajl flag instead.   SHLIB_LINK = -lz -lyajl endif

In initial tests, this worked both in Fedora 16 and OS X.

Can you try making this change in the Makefile and see if this works for you?

If this works for you, we can check this in after testing it a bit more in other platforms.

theory commented 10 years ago

Yeah, that works against 9.2, and make installcheck passes, too. Thanks.

theory commented 10 years ago

BTW, here is the fix for 9.3:

--- a/json_fdw.c
+++ b/json_fdw.c
@@ -47,7 +47,7 @@
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
-
+#include "access/htup_details.h"

 /* Local functions forward declarations */
 static StringInfo OptionNamesString(Oid currentContextId);

Thanks to @adunstan for the fix.

theory commented 10 years ago

And this will allow it to still work on 9.2:

diff --git a/json_fdw.c b/json_fdw.c
index 7e618a8..572886b 100644
--- a/json_fdw.c
+++ b/json_fdw.c
@@ -48,6 +48,9 @@
 #include "utils/memutils.h"
 #include "utils/rel.h"

+#if PG_VERSION_NUM >= 90300
+#include "access/htup_details.h"
+#endif

 /* Local functions forward declarations */
 static StringInfo OptionNamesString(Oid currentContextId);