Skip to content

OGRLayer: Fix AttributeFilterEvaluationNeedsGeometry() accounting for GeomFields of feature. - #15435

Open
wslaghekke wants to merge 1 commit into
OSGeo:masterfrom
wslaghekke:ogrlayer_filter_geom
Open

wslaghekke wants to merge 1 commit into
OSGeo:masterfrom
wslaghekke:ogrlayer_filter_geom

Conversation

@wslaghekke

@wslaghekke wslaghekke commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

The AttributeFilterEvaluationNeedsGeometry() method only accounted for the special fields and not the geometry fields themselves. Downstream effect from this in ogr_shape is that it can return the wrong count for filters that refer to geometry fields, because it used this method to skip parsing of geometry in the feature count path.

This also causes the shapefile code to return a wrong feature count if a filter refers to the geometry field, so i wrote a test to prove / validate that.
Reproduction path for that is as simple as passing it any shapefile with geometry and filtering on geometry not being null, we expect the number of features with geometry but that always returns 0 in the old code
ogrinfo -so poly.shp poly -where '"_ogr_geometry_" IS NOT NULL'

What are related issues/pull requests?

AI tool usage

  • AI (Y-a-t-il-un-Copilot-dans-l'avion, Chat-j'ai-pété, Jean-Claude Dusse or something similar) supported my development of this PR. See our policy about AI tool use. Use of AI tools must be indicated.
    Claude helped me verify this issue, i wrote all the code and ran tests.

Tasklist

  • Make sure code is correctly formatted (cf pre-commit configuration)
  • Add test case(s)
  • Add documentation
  • Updated Python API documentation (swig/include/python/docs/)
  • Review
  • Adjust for comments
  • All CI builds and checks have passed

… GeomFields of feature.

The method only accounted for the special fields and not the geometry fields themselves. Downstream effect from this in ogr_shape is that it can return the wrong count for filters that refer to geometry fields, because it used this method to skip parsing of geometry in the feature count path

This branch has not been deployed

No deployments
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