Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not hardcode in OIDs from server headers #12

Closed
daurnimator opened this issue Jan 12, 2015 · 12 comments
Closed

Do not hardcode in OIDs from server headers #12

daurnimator opened this issue Jan 12, 2015 · 12 comments

Comments

@daurnimator
Copy link
Contributor

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
Copy link
Collaborator

mbalmer commented Jan 12, 2015

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Collaborator

mbalmer commented Feb 10, 2015

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

@daurnimator
Copy link
Contributor Author

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/)
  1. 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
Copy link

kiug commented Feb 11, 2015

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:

    • /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/)

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
#12 (comment).

Karol Drożak

@mbalmer
Copy link
Collaborator

mbalmer commented Feb 11, 2015

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:

  • /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
#12 (comment).

Karol Drożak


Reply to this email directly or view it on GitHub
#12 (comment).

@daurnimator
Copy link
Contributor Author

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
Copy link
Collaborator

mbalmer commented Feb 13, 2015

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.

@mbalmer mbalmer closed this as completed Feb 13, 2015
@kiug
Copy link

kiug commented Feb 13, 2015

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
#12 (comment).

Karol Drożak

@mbalmer
Copy link
Collaborator

mbalmer commented Feb 13, 2015

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
Copy link

kiug commented Feb 13, 2015

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
#12 (comment).

Karol Drożak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants