WFS: Don't issue STARTINDEX if feature count is small #8146
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
craigds
added a commit
to koordinates/gdal
that referenced
this pull request
Jul 24, 2023
This adds the `OGR_WFS_PAGING_ALLOWED=CHECK_WITH_HITS` config option value, which causes OGR to issue a GetFeatureCount() request before deciding whether to paginate WFS layers. It doesn't work at present because it can easily cause infinite recursion: `MakeGetFeatureURL -> GetFeatureCount -> MakeGetFeatureURL` Builds on PR OSGeo#8146. Not sure this will proceed though.
craigds
added a commit
to koordinates/gdal
that referenced
this pull request
Jul 25, 2023
Backports OSGeo#8146 Datasources with no primary key cannot be naturally ordered by the server, and so using the STARTINDEX argument causes a 400 error with the response: > Cannot do natural order without a primary key, please add it or > specify a manual sort over existing attributes This change avoids issuing STARTINDEX if: * the feature count is known by the time the first GetFeature request is issued * the feature count is less than the page size. In other words, this change allows us to use small datasets that have no primary key, while still allowing paging in larger datasets (as long as they have a primary key)
|
For tests, the best is to simulate a fake server by providing GetCapabilities, DescribeFeatureCount, GetFeature responses in temporary files like done in most of the tests of ogr_wfs.py. |
craigds
added a commit
to koordinates/gdal
that referenced
this pull request
Jul 26, 2023
Backports OSGeo#8146 Datasources with no primary key cannot be naturally ordered by the server, and so using the STARTINDEX argument causes a 400 error with the response: > Cannot do natural order without a primary key, please add it or > specify a manual sort over existing attributes This change avoids issuing STARTINDEX if: * the feature count is known by the time the first GetFeature request is issued * the feature count is less than the page size. In other words, this change allows us to use small datasets that have no primary key, while still allowing paging in larger datasets (as long as they have a primary key)
Datasources with no primary key cannot be naturally ordered by the server, and so using the STARTINDEX argument causes a 400 error with the response: > Cannot do natural order without a primary key, please add it or > specify a manual sort over existing attributes This change avoids issuing STARTINDEX if: * the feature count is known by the time the first GetFeature request is issued * the feature count is less than the page size. In other words, this change allows us to use small datasets that have no primary key, while still allowing paging in larger datasets (as long as they have a primary key)
craigds
force-pushed
the
wfs-disable-paging-for-small-layers
branch
from
6575ecb
to
aeb4a11
Compare
|
I have added a test that exercises the new behaviour. |
|
The CI failures seem to be unrelated to the change I made here. Is there a known issue with CI? |
for the Mac one, now, yes :-) Due to a version upgrade of a dependency in homebrew |
elpaso
pushed a commit
to elpaso/gdal
that referenced
this pull request
Sep 4, 2023
Datasources with no primary key cannot be naturally ordered by the server, and so using the STARTINDEX argument causes a 400 error with the response: > Cannot do natural order without a primary key, please add it or > specify a manual sort over existing attributes This change avoids issuing STARTINDEX if: * the feature count is known by the time the first GetFeature request is issued * the feature count is less than the page size. In other words, this change allows us to use small datasets that have no primary key, while still allowing paging in larger datasets (as long as they have a primary key)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
WFS datasources with no primary key cannot be naturally ordered by the server, and so using the STARTINDEX argument causes a 400 error with the response:
An example is here: https://geo.irceline.be/wfs?request=GetFeature&version=2.0.0&service=WFS&TYPENAMES=bc_24hmean&COUNT=1000000&STARTINDEX=0
This change avoids issuing
STARTINDEXif:In other words, this change allows us to use small datasets that have no primary key, while still allowing paging in larger datasets (as long as they have a primary key)
What's not included
OGR_WFS_PAGING_ALLOWED(COUNT_WITH_HITS) in addition to the existing boolean values. I attempted to implement that but quickly came up against infinite recursion, since this makesMakeGetFeatureURLcallGetFeatureCountwhich in turn eventually callsMakeGetFeatureURL. I decided to skip that complication for this PR, which is still useful in most circumstances without it. The WIP change is here if you want to see it.Tests.I've now included a testWhat are related issues/pull requests?
As discussed in gdal-dev a month ago
Tasklist
Environment
Provide environment details, if relevant: