Skip to content

Deserialize geometry #300

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

manticore-projects
Copy link

@manticore-projects manticore-projects commented Jul 6, 2025

Greetings!

This fixes the ResultSet.getString() method for GEOMETRY and fixes #298:

select ST_GeomFromGeoJSON('{
      "type": "Point",
      "coordinates": [30.0, 10.0]}') as p;

since 1.2.2 was shown as:

DuckDBBlobResult{buffer=java.nio.HeapByteBuffer[pos=0 lim=32 cap=32]}

now with the deserialization applied it becomes again

POINT (30 10)

(Beside that I fixed two TIMEZONE related tests that would not work in other timezones (e.g. failed when system timezone is ASIA/BANGKOK).

Signed-off-by: manticore-projects <[email protected]>
@staticlibs
Copy link
Collaborator

Hi, thanks for the PR! You can fix formatting problems by running:

python ./scripts/format.py

Though I am not sure that adding DuckDBGeometryDeserializer to the driver itself is a good idea, in general we try not to have any extension-specific code in JDBC. I wonder if this logic can go into client app/connector part instead?

@manticore-projects
Copy link
Author

manticore-projects commented Jul 6, 2025

Greetings!

Thank you for your feedback. I have applied the formatter.

On the implementation:

  • I don't think it belongs to the client because client because client will normally see Object or Blob or String and then simply does not know what to do with it. Also, until 1.2.2 everything was working fine. What broke it is the ResultSet.getString() applying Object.toString() on just everything including the Blob.
    Example: CSVWriter is a handy library to quickly turn a ResultSet into ASCII/CSV to verify the content (in Unit Tests). CSVWriter is of course RDBMS agnostic and uses simple JDBC features only. This worked great with DuckDB 1.2.1 but is broken now for GEOMETRY. Shall every single JDBC tool in the world implement now a special treatment for DuckDB GEOMETRY?

  • DuckDB C code has a similar de-serializer, its just not wrapped/ported (and so I provided the port)

  • I am not sure, what exactly you mean by connector part

I agree with you that the need for the de-serializer is sub-optimal. But in my understanding, there are only two options:

  1. implement the GEOMETRY class in Java (like you did for ARRAY and STRUCT and BLOB)
  2. or implement the Deserializer

As far as I see, Option 1) was worse because you either reinvent the wheel or start depending on 3rd party libraries like JTS.

@staticlibs
Copy link
Collaborator

@manticore-projects

I am not sure, what exactly you mean by connector part

I meant some kind of intermediate code like org.jkiss.dbeaver.ext.duckdb.model.DuckDBSQLDialect.

I will follow-up on this after 1.3.2 release next week.

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

Successfully merging this pull request may close these issues.

[DuckDB-1.3.1.0] Geometry Functions return now DuckDBBlobResult instead of Text
2 participants