search

arcapos / luapgsql

Lua binding for PostgreSQL
108 stars 24 forks source link

Do not hardcode in OIDs from server headers #12

Closed daurnimator closed 9 years ago

daurnimator commented 9 years ago

They can be changed per server. They must be fetched per connection with SELECT 'typename'::regtype::oid; They may/should be cached (per connection)

This will be tricky to do non-blockingly.

mbalmer commented 9 years ago

I discussed this issue on the PostgreSQL IRC channel. The issue, while true in theory, is not of a practical relevance here:

"The server's headers have the correct oids for that version of the server. Fortunately, for the basic types like int/bool/char/text/numeric/timestamp/float the oids have not changed in any pg version you'd want to support, also refcursor, regclass and regtype, void, unknown are probably safe to assume are stable.

The danger is with types that were more recently adopted, which might show up on older versions as an extension type, e.g. before there was a builtin uuid type there were extensions for it, and possibly also for json.

This is the exact list from a "conservative" list, i.e. stuff which can't be changed without breaking half the clients that already exist: boolean, character (bpchar), character varying (varchar), text, bytea, double precision (float8), real (float4), numeric, smallint, integer, bigint, date, timestamp, timestamp with time zone, time, time with time zone, interval, refcursor, regclass, regtype, void, unknown.

The non-conservative list would be the list of #defines from catalog/pg_type.h from the oldest pg version you plan to support many clients assume that the oids for array types are fixed, for example." (RhodiumToad on #postgresql)

So I suggest to close this issue.

daurnimator commented 9 years ago

The non-conservative list would be the list of #defines from catalog/pg_type.h from the oldest pg version you plan to support many clients assume that the oids for array types are fixed, for example." (RhodiumToad on #postgresql)

So I suggest to close this issue.

In that case, could we move the OID defines into this project, so that the postgres server headers do not need to be installed to compile luapgsql?

daurnimator commented 9 years ago

I ran into this issue pretty badly today; it looks like the debian packages have broken postgres_fe.h

Can we move the OID defines into the project so we can drop the dependency on that header?

mbalmer commented 9 years ago

How did you run into this issue? I am having no problems neither on squeeze, wheezy, nor on an Ubuntu variant.

daurnimator commented 9 years ago

How did you run into this issue? I am having no problems neither on squeeze, wheezy, nor on an Ubuntu variant.

So I'm trying to run on a server running ubuntu precise (12.04). The first issue is that luapgsql needs PQsetSingleRowMode, which is only in 9.2+. So, I upgraded to 9.4 using the repo at http://www.postgresql.org/download/linux/ubuntu/ However, their includes seem to be broken:

    • /usr/include/postgresql/internal/postgres_fe.h #includes "c.h"
    • /usr/include/postgresql/internal/c.h #includes "postgres_ext.h
    • postgres_ext.h is in /usr/include/postgresql, (WITHOUT internal/)
  2. If I hack around the above, I get
    • /usr/include/postgresql/internal/postgres_fe.h:27:32: fatal error: common/fe_memutils.h: No such file or directory
    • This file is not in the packages at all.

These are the currently installed packages:

$ dpkg -l | grep ostgre
ii  libpq-dev                        9.4.1-1.pgdg12.4+1                header files for libpq5 (PostgreSQL library)
ii  libpq5                           9.4.1-1.pgdg12.4+1                PostgreSQL C client library
ii  pgdg-keyring                     2014.1                            keyring for apt.postgresql.org
ii  postgresql                       9.4+166.pgdg12.4+1                object-relational SQL database (supported version)
ii  postgresql-9.1                   9.1.15-1.pgdg12.4+1               object-relational SQL database, version 9.1 server
ii  postgresql-9.4                   9.4.1-1.pgdg12.4+1                object-relational SQL database, version 9.4 server
ii  postgresql-client-9.1            9.1.15-1.pgdg12.4+1               front-end programs for PostgreSQL 9.1
ii  postgresql-client-9.4            9.4.1-1.pgdg12.4+1                front-end programs for PostgreSQL 9.4
ii  postgresql-client-common         166.pgdg12.4+1                    manager for multiple PostgreSQL client versions
ii  postgresql-common                166.pgdg12.4+1                    PostgreSQL database-cluster manager
kiug commented 9 years ago

Under the Windows with PostgreSQL installed from binary package ( http://www.enterprisedb.com/postgresql-941-binaries-win32?ls=Crossover&type=Crossover) is the same problem.

2015-02-10 16:38 GMT+01:00 daurnimator notifications@github.com:

How did you run into this issue? I am having no problems neither on squeeze, wheezy, nor on an Ubuntu variant.

So I'm trying to run on a server running ubuntu precise (12.04). The first issue is that luapgsql needs PQsetSingleRowMode, which is only in 9.2+. So, I upgraded to 9.4 using the repo at http://www.postgresql.org/download/linux/ubuntu/ However, their includes seem to be broken:

1.

  • /usr/include/postgresql/internal/postgres_fe.h #includes "c.h"
  • /usr/include/postgresql/internal/c.h #includes "postgres_ext.h

  • postgres_ext.h is in /usr/include/postgresql, (WITHOUT internal/) 2.

    If I hack around the above, I get

    • /usr/include/postgresql/internal/postgres_fe.h:27:32: fatal error: common/fe_memutils.h: No such file or directory
  • This file is not in the packages at all.

These are the currently installed packages:

$ dpkg -l | grep ostgre ii libpq-dev 9.4.1-1.pgdg12.4+1 header files for libpq5 (PostgreSQL library) ii libpq5 9.4.1-1.pgdg12.4+1 PostgreSQL C client library ii pgdg-keyring 2014.1 keyring for apt.postgresql.org ii postgresql 9.4+166.pgdg12.4+1 object-relational SQL database (supported version) ii postgresql-9.1 9.1.15-1.pgdg12.4+1 object-relational SQL database, version 9.1 server ii postgresql-9.4 9.4.1-1.pgdg12.4+1 object-relational SQL database, version 9.4 server ii postgresql-client-9.1 9.1.15-1.pgdg12.4+1 front-end programs for PostgreSQL 9.1 ii postgresql-client-9.4 9.4.1-1.pgdg12.4+1 front-end programs for PostgreSQL 9.4 ii postgresql-client-common 166.pgdg12.4+1 manager for multiple PostgreSQL client versions ii postgresql-common 166.pgdg12.4+1 PostgreSQL database-cluster manager

— Reply to this email directly or view it on GitHub https://github.com/mbalmer/luapgsql/issues/12#issuecomment-73720499.

Karol Drożak

mbalmer commented 9 years ago

It will be changed withing a few days. I will include the few OID definitions directly in luapgsql.h. Should happen this weekend latest.

Am 11.02.15 um 09:18 schrieb Karol Drozak:

Under the Windows with PostgreSQL installed from binary package ( http://www.enterprisedb.com/postgresql-941-binaries-win32?ls=Crossover&type=Crossover) is the same problem.

2015-02-10 16:38 GMT+01:00 daurnimator notifications@github.com:

How did you run into this issue? I am having no problems neither on squeeze, wheezy, nor on an Ubuntu variant.

So I'm trying to run on a server running ubuntu precise (12.04). The first issue is that luapgsql needs PQsetSingleRowMode, which is only in 9.2+. So, I upgraded to 9.4 using the repo at http://www.postgresql.org/download/linux/ubuntu/ However, their includes seem to be broken:

1.

  • /usr/include/postgresql/internal/postgres_fe.h #includes "c.h"
  • /usr/include/postgresql/internal/c.h #includes "postgres_ext.h
  • postgres_ext.h is in /usr/include/postgresql, (WITHOUT internal/) 2.

If I hack around the above, I get

  • /usr/include/postgresql/internal/postgres_fe.h:27:32: fatal error: common/fe_memutils.h: No such file or directory
  • This file is not in the packages at all.

These are the currently installed packages:

$ dpkg -l | grep ostgre ii libpq-dev 9.4.1-1.pgdg12.4+1 header files for libpq5 (PostgreSQL library) ii libpq5 9.4.1-1.pgdg12.4+1 PostgreSQL C client library ii pgdg-keyring 2014.1 keyring for apt.postgresql.org ii postgresql 9.4+166.pgdg12.4+1 object-relational SQL database (supported version) ii postgresql-9.1 9.1.15-1.pgdg12.4+1 object-relational SQL database, version 9.1 server ii postgresql-9.4 9.4.1-1.pgdg12.4+1 object-relational SQL database, version 9.4 server ii postgresql-client-9.1 9.1.15-1.pgdg12.4+1 front-end programs for PostgreSQL 9.1 ii postgresql-client-9.4 9.4.1-1.pgdg12.4+1 front-end programs for PostgreSQL 9.4 ii postgresql-client-common 166.pgdg12.4+1 manager for multiple PostgreSQL client versions ii postgresql-common 166.pgdg12.4+1 PostgreSQL database-cluster manager

— Reply to this email directly or view it on GitHub https://github.com/mbalmer/luapgsql/issues/12#issuecomment-73720499.

Karol Drożak

— Reply to this email directly or view it on GitHub https://github.com/mbalmer/luapgsql/issues/12#issuecomment-73846395.

daurnimator commented 9 years ago

It will be changed withing a few days. I will include the few OID definitions directly in luapgsql.h. Should happen this weekend latest.

How did discussion with postgres devs go?

mbalmer commented 9 years ago

Did not discuss the issue at all. But I fixed it an rearranged the headers so that it compile without the server header files being installed.

kiug commented 9 years ago

For conditionally compile should not be PG_VERSION_NUM instead PG_VERSION_NUMBER?

2015-02-13 10:52 GMT+01:00 Marc Balmer notifications@github.com:

Did not discuss the issue at all. But I fixed it an rearranged the headers so that it compile without the server header files being installed.

— Reply to this email directly or view it on GitHub https://github.com/mbalmer/luapgsql/issues/12#issuecomment-74229463.

Karol Drożak

mbalmer commented 9 years ago

Am 13.02.15 um 12:05 schrieb Karol Drozak:

For conditionally compile should not be PG_VERSION_NUM instead PG_VERSION_NUMBER?

Of course! Thanks for letting me know. I fixed it on github.

kiug commented 9 years ago

Take a look at luapgsql_win.patch, maybe you will be interested.

2015-02-13 13:08 GMT+01:00 Marc Balmer notifications@github.com:

Am 13.02.15 um 12:05 schrieb Karol Drozak:

For conditionally compile should not be PG_VERSION_NUM instead PG_VERSION_NUMBER?

Of course! Thanks for letting me know. I fixed it on github.

— Reply to this email directly or view it on GitHub https://github.com/mbalmer/luapgsql/issues/12#issuecomment-74244481.

Karol Drożak