Skip to content

Don't error on later GeoParquet versions#19243

Closed
Maxxen wants to merge 1 commit intoduckdb:v1.4-andiumfrom
Maxxen:gpq-fix
Closed

Don't error on later GeoParquet versions#19243
Maxxen wants to merge 1 commit intoduckdb:v1.4-andiumfrom
Maxxen:gpq-fix

Conversation

@Maxxen
Copy link
Member

@Maxxen Maxxen commented Oct 2, 2025

Given that GeoParquet 2.0 is around the corner and we already accept native geometry types, this seems like a bad idea.

@Maxxen Maxxen closed this Oct 3, 2025
Mytherin added a commit that referenced this pull request Oct 3, 2025
…ol GeoParquet version (#19244)

In #18832 we added support for
reading and writing geometry as native parquet geometry types. I also
made it so that we write native geometry types by default, always. I
thought this would be safe to do as the new parquet geometry native type
is just a logical type, so older parquet readers that don't support it
yet would just continue to read it like any other blob.

Unfortunately, as shown in
duckdb/duckdb-spatial#688 (comment)
this turns out to be wrong, lots of readers throw errorsinstead of
ignoring the logical type annotation.

This PR thus changes the behavior back to writing blobs + geoparquet
metadata, but also adds a new `GEOPARQUET_VERSION` flag to select what
geoparquet version to use. Unfortunately again, GeoParquet 2.0 is
supposed to be based on the new native geometry type, but there is no
official standard released yet. Therefore, the valid options are as
follows:

- `NONE` = Don't write GeoParquet metadata, store geometries as native
parquet geometry logical type with geo stats

- `V1` = Write GeoParquet metadata, store geometries as blobs with no
stats.
  Made default in this PR, same behavior as DuckDB v1.3.2
  
- `BOTH` = Write GeoParquet metadata, _and_ store geometries as native
parquet geometry logical type with stats
  Accidentally made default in v1.4.0
  
Hopefully `NONE` or a future `V2` can be default in DuckDB v1.5 together
with the larger geometry overhaul.

This is an actual fix compared to
#19243,

Ideally this gets in as part of v1.4.1
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.

1 participant