Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#5309 closed defect (fixed)

[PATCH] OGRLayer::GetFeature() does not clear filters

Reported by: warmerdam Owned by: warmerdam
Priority: normal Milestone: 1.11.0
Component: OGR_SF Version: unspecified
Severity: normal Keywords:
Cc:

Description

Calling GetFeature(id) after setting filters will not get that feature if it is excluded by the filters. I am not convinced this is the correct behavior.

NOTE: this is in the situation of using the default OGRLayer::GetFeature() implementation.

Attachments (1)

ticket5309.patch (16.1 KB ) - added by Even Rouault 10 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 by Even Rouault, 10 years ago

I also agree, and this behaviour goes against the documentation of the function : "Success or failure of this operation is unaffected by the spatial or attribute filters."

Ideally we should clear the filters when entering OGRLayer::GetFeature() and restoring them when exiting, but for implementations that have overridden SetAttributeFilter() and/or SetSpatialFilter() and don't call the base methods, I don't know how we can do the restauration.

comment:2 by Even Rouault, 10 years ago

Summary: OGRLayer::GetFeature() does not clear filters[PATCH] OGRLayer::GetFeature() does not clear filters

Attached a proposed fixed for that issue :

  • it adds a m_pszAttrQueryString attribute in OGRLayer that is set by OGRLayer::SetAttributeFilter(), and must be set by specialized implementations that don't fallback on OGRLayer::SetAttributeFilter()
  • OGRLayer::GetFeature(): unset attribute&spatial filters, and restore them back
  • test_ogrsf: add testing that attribute&spatial filters are ignored by GetFeature(), and well restored afterwards
  • fix OGDI GetFeature() so that it doesn't evaluate attribute&spatial filters
  • make PG, MySQL, MSSQLSpatial, OCI, SDE, PGeo, ODBC, WALK, IDB, SQLite and INGRES driver set the m_pszAttrQueryString attribute in their SetAttributeFilter() implementation. For those drivers, the generic GetFeature() is called on tables without feature id column that use, so if not setting m_pszAttrQueryString, the generic GetFeature() wouldn't be able to restore the original attribute filter

by Even Rouault, 10 years ago

Attachment: ticket5309.patch added

comment:3 by warmerdam, 10 years ago

Wow, sounds good. Don't hesitate to apply it.

I think it's worth sending an email clarifying the behavior expectations to the mailing list and perhaps make it clear in the GetFeature() docs that it is not / should not be affected by filters in place.

Thanks!

comment:4 by Even Rouault, 10 years ago

Milestone: 2.0
Resolution: fixed
Status: newclosed

trunk r26688 "GetFeature(): make sure that the behaviour is not influenced by attribute or spatial filters in the generic implementation (by adding a m_pszAttrQueryString layer member to properly save/restore attribute filter); upgrade OGDI, PG, MySQL, MSSQLSpatial, OCI, SDE, PGeo, ODBC, WALK, IDB, SQLite and Ingres driver to be compliant with expected behaviour; make test_ogrsf test the intended behaviour (#5309)"

comment:5 by Even Rouault, 10 years ago

Milestone: 2.01.11.0
Note: See TracTickets for help on using tickets.