Postgis+Hibernate support

[apotapov]

I was planning to use pgjdbc-ng when I moved from BoneCP to HikariCP. (recommended by HikariCP) However, I got the following exception when I was trying to retrieve data with postgis fields using Hibernate.

Relevant hibernate properties: hibernate.dialect=org.hibernate.spatial.dialect.postgis.PostgisDialect

Exception:

! java.lang.IllegalArgumentException: Can't convert object of type java.lang.String
! at org.hibernate.spatial.dialect.postgis.PGGeometryValueExtractor.toJTS(PGGeometryValueExtractor.java:113) ~[batgirl.jar:na]
! at org.hibernate.spatial.dialect.AbstractJTSGeometryValueExtractor.doExtract(AbstractJTSGeometryValueExtractor.java:50) ~[batgirl.jar:na]

This works fine when using default Postgres JDBC data source: org.postgresql.ds.PGSimpleDataSource

[apotapov]

Postgis field in my database:

SELECT AddGeometryColumn('users','coords','4326','POINT',2);

Value of the particular coords field the application was trying to extract: 0101000020E6100000D8FB43E9C1C858C03EA1E8CB1C6E3340

[brettwooldridge]

Can you attach a schema fragment from the table in question?

[apotapov]

Sure

                                          Table "public.users"
       Column        |            Type             |                     Modifiers
---------------------+-----------------------------+----------------------------------------------------
 id                  | bigint                      | not null default nextval('users_id_seq'::regclass)
 created             | timestamp without time zone |
 updated             | timestamp without time zone |
 access_token        | text                        |
 active              | boolean                     | not null default true
 birthday            | timestamp without time zone |
 test_user           | boolean                     | not null default true
 email               | character varying(63)       |
 facebook_id         | bigint                      | not null
 first_name          | character varying(127)      |
 gender              | character varying(31)       |
 last_login          | timestamp without time zone |
 last_name           | character varying(127)      |
 locale              | character varying(127)      |
 location            | character varying(127)      |
 name                | character varying(255)      |
 profile_link        | character varying(1023)     |
 relationship_status | character varying(31)       |
 role                | character varying(15)       | not null
 timezone            | double precision            |
 token_expiration    | bigint                      |
 username            | character varying(255)      |
 lastanswered_id     | bigint                      |
 play_music          | boolean                     | not null default true
 play_sound          | boolean                     | not null default true
 show_age            | boolean                     | not null default true
 lastplayed_id       | bigint                      |
 coords              | geometry(Point,4326)        |
 password            | character varying(255)      |
 created_by          | bigint                      |
 updated_by          | bigint                      |
 image_id            | bigint                      |
 likes_men           | boolean                     | not null default false
 likes_women         | boolean                     | not null default false
Indexes:
    "users_pkey" PRIMARY KEY, btree (id)
    "uk_jmubronqnn4q0cwe2egqsgvnl" UNIQUE CONSTRAINT, btree (facebook_id)
Foreign-key constraints:
    "fk_1mm87ps8glleprgwytvy0u0hk" FOREIGN KEY (lastanswered_id) REFERENCES questions(id)
    "fk_1mm87ps8glleqqgwytvy0u0hk" FOREIGN KEY (lastplayed_id) REFERENCES games(id)
    "users_created_by_key" FOREIGN KEY (created_by) REFERENCES users(id)
    "users_image_key" FOREIGN KEY (image_id) REFERENCES images(id)
    "users_updated_by_key" FOREIGN KEY (updated_by) REFERENCES users(id)

[brettwooldridge]

@kdubb After digging into this a little, I realized I know little to nothing about how pgsql-ng maps unknown types like PGgeometry (from org.postgis), etc.

I did find the Hibernate Spatial tutorial here. All you need to do is:

1. Clone it
2. Edit the pom.xml to add pgjdbc-ng
3. Edit src/main/resources/hibernate.cfg.xml to use jdbc:pgsql rather than jdbc:postgresql
4. In your 'test' database run 'CREATE EXTENSION postgis'
5. mvn clean compile
6. mvn exec:java -Dexec.mainClass="event.EventManager" -Dexec.args="store POINT(10 5)"

In step 6, you can replace 'mvn' with 'mvnDebug' to attach a debugger.

Currently, running the tutorial generates:

Caused by: java.lang.NullPointerException
    at com.impossibl.postgres.jdbc.SQLTypeUtils.mapSetType(SQLTypeUtils.java:84)
    at com.impossibl.postgres.jdbc.PGPreparedStatement.coerceParameters(PGPreparedStatement.java:216)
    at com.impossibl.postgres.jdbc.PGPreparedStatement.addBatch(PGPreparedStatement.java:280)
    at org.hibernate.jdbc.BatchingBatcher.addToBatch(BatchingBatcher.java:53)

Which is different from above, but I suspect is masking the above error. The value that is throwing an exception here is of type org.postgis.PGgeometry, and because we find no type mapping an NPE is eventually generated.

[brettwooldridge]

@kdubb I'm looking at this one a little more. I'm still way over my head in terms of what the implementation is doing, so I have a few questions:

It's seems that no type information is found for a PostGIS "geometry" type. Looking at the code, I'm following the Registry initialization. It seems that:

• loadModifierParser() is called with modInId=113201 and modOutId=113202
• lookupProcName() is called and returns "geometry_typemod_in"
• Procs.loadModiferParseProc() is then called with this name
• a for loop iterates over Procs.PROVIDERS, calling findModifierParser() on each

None of the ProcProviders match. In order to support PostGIS, do we need a special provider for Geometry?

[kdubb]

Yes. Without looking I'm assuming we'll need a TextEncoder and BinaryEncoder for the geometry type as well.

The way I did this for the rest of the types is to simply adapt the code from the server itself. In this case the functions are probably in src/backend/utils/adt/geo_ops.c

I'll look into this right now and report back some more.

[kdubb]

OK. So I looked into this a bit more. After the realization that we are discussing the postgis code (which is a separate project) I identified the methods we need to implement and their location.

Basically to implement this type we need to make a Codec.Encoder and Codec.Decoder for text (usually called TextEncoder, TextDecoder) and binary (usually called BinaryEncoder, BinaryDecoder). To follow the current format these are implemented as sub-classes inside a ProcProvider. The ProcProvider is what is also what is used to wire the encoders/decoders into the type system; the SimpleProcProvider wires them up by name and is the general implementation to use.

Basic steps we need to take...

1. Recreate LWGEOM_in as a TextEncoder
2. Recreate LWGEOM_out as a TextDecoder
3. Recreate LWGEOM_recv as a BinaryEncoder
4. Recreate LWGEOM_send as a BinaryEncoder
5. Implement a SimpleProcProvider passing these encoders/decoders

The interesting news is that although the LWGEOM_* functions look pretty complicated they might already be implemented in java in postgis/java/jdbc/src/org/postgis/binary BinaryParser and BinaryWriter as well as text parsers scattered in related packages.

[brettwooldridge]

Should be care at all about a dependency on the org.postgis packages/classes? Seems rather unavoidable if we have to produce objects as outputs in some cases (eg ResultSet.getObject()) .

[kdubb]

If the maven artifact (I found org.postgis:postgis-stubs) is up to date I don't see why we wouldn't just add it. If it's not... maybe just copy/paste the most recent java code from the project.

[brettwooldridge]

I guess a meant a runtime dependency. Users will not be able to use pgjdbc-ng without the postgis jar present. I'm not sure how PostgreSQL deals with this, possibly shading the classes?

[kdubb]

I hadn't thought about this. Yes, we must shade or copy them in manually to avoid this.

[brettwooldridge]

I've been thinking about it more, and I think that shading and/or copying will not work. If the user has postgis classes in their classloader, and that classloader is not the same as ours, then postgis objects passed into and out of pgjdbc-ng will not be seen as being of the same "class" and will fail at runtime.

I think the right approach is to move the loading of SimpleProcProvider instances to a dynamic model (using ClassLoader.loadClass()) rather than a statically defined list in Procs. In this way, if a postgis type is discovered at runtime, the appropriate SimpleProcProvider would be loaded with the assumption that the postgis classes will be present. If there are no postgis types discovered, then the SimpleProcProviders with dependences on postgis will never be loaded -- removing the runtime dependency.

All we need to do in that case is, as you said, have a compile dependency in our pom.

[kdubb]

Procs.PROVIDERS was originally loaded using ServiceLoader. I had an issue when I loaded the driver in a container (e.g. Tomcat). The services file is actually still in the META-INF folder.

This coincides with using a "compile" time dependency as well; since ServiceLoader fails gracefully when it cannot load a particular implementation of a service interface.

I think reverting back to this method is probably the best idea... as long as it works in containers.

[kdubb]

@brettwooldridge I committed changes to the Procs. It's now an instance that uses the Driver's class loader to build a list of ProcProviders using the Service mechanism. This appears to have fixed all issues I encountered previously with using this mechanism in a container (aka multi-classloader) environment.

This paves the way for easy support for postgis and other extensions. Now we can make a separate artifact that depends on the postgis code and plugs into the driver with ease.

I have downloaded the postgis code and built the jar. They are doing quite a bit of driver integration already; it might be interesting to see if they'd just include the ProcProvider implementation in their own code.

[frode-carlsen]

What's the current status on pgjdbc-ng postgis support?
Happy to help out if need be.

[jesperpedersen]

@frode-carlsen I don't think anybody is working on that, so feel free to give it a shoot

[frode-carlsen]

I'll give it a shot.

From your previous last comment , I've set it up as a separate maven artifact with a dependency on bot pgjdbc-ng and org.postgis. I take it the idea was to implement SimpleProcProviders and add them using the service loader mechanism in the jar?

btw: think I found a couple of issues with the test set:

1. following tests are sensitive to the EOL setting on the OS: com.impossibl.postgres.jdbc.BlobTest#testGetBytesOffsetBlob and #testGetBytesOffsetBlob. They assume EOL is char(10), and can't handle chr(10)chr(13) on windows.
2. CodecTests regarding interval fails. This seems to be due to the Interval class always expecting the decimal separator to be '.' when parsing, but outputting with the platform default decimal separator in its toString method (which on my system is comma - using the no_NO locale)

[jesperpedersen]

@frode-carlsen Ok, better submit a PR for the test suite failures on your machine first then. Yes, a service mechanism must be used for this, as the "core" JDBC driver shouldn't require Postgist. Feel free to submit a PR with your initial work in order to get feedback on the approach. You can always add commits, and squash in the end :)

[frode-carlsen]

@jesperpedersen I've set up a basic provider for postgis geometry datatypes and codec tests here: https://github.com/frode-carlsen/pgjdbc-ng-postgis

Care to take a look and see if the structure agrees with you? There are some datatypes that aren't done (bbox, etc.), but initial geometry codec is there. It relies on a PR I just submitted with a couple of tiny tweaks to pgjdbc-ng.

It would be nice to have version numbers align with pgjdbc-ng, but that can't be enforced unless they share same parent (or this becomes a child of existing). Would you prefer keeping it as a separate repository, or do you foresee it as a submodule of pgjdbc-ng?

Also, doing this work,I've come to realise that what I really want (and by implication of course everyone else too), is a straight to JTS (Java Topology Suite) conversion. When this gets completed, I think I'll want to look into that.

[brettwooldridge]

@frode-carlsen @jesperpedersen I don't see any reason why postgis support should not be a "part of" pgjdbc-ng, just as it is with the official PostgreSQL driver. The only requirement that the official driver imposes is that the user needs the postgis-jdbc JAR file. If the implementation is reflection-based then there is only a compile-time dependency, but no runtime-dependency is imposed on non-Postgis users.

[frode-carlsen]

@brettwooldridge do you mean to include it as part of the core jar, with reflection based check to load the codec? I'm happy to do that. Today it's a service provider that loads the types, in this case it would need to add support to check to see if the type should be added (using a factory approach), or swallow the exception generated if class initialization exception.

[brettwooldridge]

@frode-carlsen Your com.impossibl.postgres.system.procs.ProcProvider file would be added to the pgjdbc-ng src/main/resources/META-INF/services directory, but renamed to something like com.impossibl.postgres.postgis.ProcProvider.

Maybe @jesperpedersen can jump in here with his thoughts on runtime dependency, but looking at the implementation of Geometrys, as far as I can tell, no runtime dependency would be incurred unless the pgjdbc-ng had the need to resolve a Postgis type.

Import dependencies are "compile time" only, and classes are only loaded when:

• they are referenced on a specific line of code during execution, or
• they are referenced in the signature of a method

Obviously, the signatures of the methods in the TxtDecoder, TxtEncoder, BinDecoder, and BinEncoder implementations make no reference to Postgis classes. So, only an actual call to encode/decode a Postgis type should create a classload demand for a Postgis class.

Beyond that, my only critique at first glance would be to rename Geometrys to PostgisProcProvider.

Oh, and of course you would need to add a dependency for postgis to the pgjdbc-ng pom:

 <dependency>
   <groupId>org.postgis</groupId>
   <artifactId>postgis-jdbc</artifactId>
   <version>1.3.3</version>
   <scope>provided</scope>
   <optional>true</optional>
</dependency>

[frode-carlsen]

@brettwooldridge can do. Unfortunately the latest postgis-jdbc versions (2.1.7) isn't available in the maven repo. I think it'd be a good thing to compile against a recent version (although they seem largely compatible), but the only way I know to get around that would be to add the latest jar as a system scope dependency for compilation and tests. The user will still need to set it up at runtime in his classpath

Also, there's another twist, turns out postgis-jdbc isn't completely independent of postgres jdbc driver (the irony that the postgis-stubs library still requires the postgres jdbc driver). Anyway, what's required are the PGtokenizer and PGobject classes. They are pretty simple and have no dependencies so can be copy-pasted as-is into to pgjdc-ng, but must be under org.postgresql.util package name. If that's ok, I'll do that.

[jesperpedersen]

@frode-carlsen If PostGis requires org.postgresql classes that needs to sorted first in the PostGis project. I see the lack of a Maven deployment as a blocker too. The rest of #178 is minor stuff that can be quickly resolved

[zandrei]

What is the status of PostGis support in pgjdbc-ng? I am trying to use it in a project with postgresql 9.3 version, pgjdbc-ng 0.5 version but I am getting the following error for geometry type:

Caused by: java.lang.IllegalStateException: type has no supported parameter format
    at com.impossibl.postgres.types.Type.getParameterFormat(Type.java:307)
    at com.impossibl.postgres.protocol.v30.ProtocolImpl.lengthOfParams(ProtocolImpl.java:723)
    at com.impossibl.postgres.protocol.v30.ProtocolImpl.writeBind(ProtocolImpl.java:508)
    at com.impossibl.postgres.protocol.v30.BindExecCommandImpl.execute(BindExecCommandImpl.java:319)

[jesperpedersen]

@zandrei It was blocked on a hard dependency on the official driver. Maybe @frode-carlsen has an update on the matter

[frode-carlsen]

I've abandoned trying any further to port the postgis library to make it work with pgjdbc-ng, for the reasons I've outlined above. Either an ugly hack is added to support it, or the official driver somehow changes it's datatype extension mechanism (avoiding the PGObject inheritance).

In any case, I believe a better solution here would be to add support for the Java Topology Suite (JTS) data types. It's pretty much universally used among all Java GIS libraries of note, and doing this will avoid a level of transformation. It's also what Hibernate Spatial ultimately produces (after going through the postgis data types), but doing it at this level will be faster, and largely remove the need for Hibernate Spatial as far as mapping data types. (it would be trivial to add hibernate mapping if the driver already returns the proper types).

A proof-of-concept for this (works with 0.6-SNAPSHOT as of today) is available here: https://github.com/frode-carlsen/pgjdbc-ng-jts

[jesperpedersen]

Thanks for the update Frode

[yeroc]

Does anyone know whether a ticket was ever opened against the PostGIS project to remove the dependency on the official JDBC driver or is that no longer the blocker to getting this supported?