Skip to content

fix the geometry of a column a sub-decoder derives from an expression - #619

Merged
azahnen merged 1 commit into
masterfrom
sql-geometry-expression-encoding
Aug 25, 2026
Merged

fix the geometry of a column a sub-decoder derives from an expression#619
azahnen merged 1 commit into
masterfrom
sql-geometry-expression-encoding

Conversation

@cportele

Copy link
Copy Markdown
Contributor

This PR fixes an unrelated issue for an edge case (geometry properties with an SQL expression) that was discovered while working on #618. The changes build on top of #618.

SqlQueryColumnOperations told the dialect whether the expression yields a geometry by passing "the column has the WKT operation". A geometry column carries exactly one of the WKT and WKB operations, chosen by queryGeneration.geometryEncoding, so with the default encoding of WKB the dialect was told the expression is not a geometry and returned it unwrapped. Every other consumer of these operations looks at both.

Three defects followed from that, in both the PostGIS and the GeoPackage dialect:

  • a geometry was returned as the raw value of the database instead of the requested encoding
  • the spatial extent query excludes the WKT and WKB operations because it wraps the raw geometry itself, but the exclusion was not honoured, so the geometry was wrapped twice
  • forcePolygonCCW and linearizeCurves were not applied, so the same geometry came back differently depending on whether it was reached through an expression

The dialect is now given the operation instead of a flag, along with the two geometry options. It cannot hold the encoding itself: it is shared by all providers of its DBMS, while the encoding is set per provider, which is why the field that used to be consulted for it was never assigned.

Both dialects implemented the method identically, so it moves to the interface, where it calls the WKT and WKB rendering of the dialect.

For correctly configured geometry expressions, the Oracle and DuckDB dialects are handled properly.

@cportele
cportele requested a review from azahnen as a code owner August 25, 2026 07:59
@cportele cportele added the bug label Aug 25, 2026
@cportele
cportele force-pushed the sql-geometry-expression-encoding branch from 012203b to ffc0a63 Compare August 25, 2026 08:00
Base automatically changed from gpkg-spatial-index to master August 25, 2026 13:01
…expression

SqlQueryColumnOperations told the dialect whether the expression yields a
geometry by passing "the column has the WKT operation". A geometry column
carries exactly one of the WKT and WKB operations, chosen by
queryGeneration.geometryEncoding, so with the default encoding of WKB the
dialect was told the expression is not a geometry and returned it
unwrapped. Every other consumer of these operations looks at both.

Three defects followed from that, in both the PostGIS and the GeoPackage
dialect:

- a geometry was returned as the raw value of the database instead of the
  requested encoding
- the spatial extent query excludes the WKT and WKB operations because it
  wraps the raw geometry itself, but the exclusion was not honoured, so
  the geometry was wrapped twice
- forcePolygonCCW and linearizeCurves were not applied, so the same
  geometry came back differently depending on whether it was reached
  through an expression

The dialect is now given the operation instead of a flag, along with the
two geometry options. It cannot hold the encoding itself: it is shared by
all providers of its DBMS, while the encoding is set per provider, which
is why the field that used to be consulted for it was never assigned.

Both dialects implemented the method identically, so it moves to the
interface, where it calls the WKT and WKB rendering of the dialect.
@azahnen
azahnen force-pushed the sql-geometry-expression-encoding branch from ffc0a63 to c4bb374 Compare August 25, 2026 13:01
@azahnen
azahnen merged commit 0dab2e0 into master Aug 25, 2026
3 checks passed
@azahnen
azahnen deleted the sql-geometry-expression-encoding branch August 25, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants