Opened 4 years ago

Closed 4 years ago

Last modified 4 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 4 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 4 years ago by Even Rouault

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 Changed 4 years ago by Even Rouault

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

Changed 4 years ago by Even Rouault

Attachment: ticket5309.patch added

comment:3 Changed 4 years ago by warmerdam

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 Changed 4 years ago by Even Rouault

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 Changed 4 years ago by Even Rouault

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