Some rationale on this change...
Previously when applying a "regular" subset string, ie. one that is only the
content of a where clause, we issued a full "SELECT * FROM layer WHERE subsetstring",
resulting in a OGR SQL layer. The caveat of that is that most OGR drivers
will have issues retaining the original FID. A hack consisting in adding a
{original_fid_name} as orig_ogc_fid to the select columns was introduced in
4ce2cf1744
to try to retain the original FID, but this added a lot of complexity. And
actually, in the case of the OGR GPKG driver, it caused it to still be confused
when analyzing the column definition of the resulting layer, since it sees
2 FID columns despite the renaming (one included in the '*' wildcard, and the
one of orig_ogc_fid), which caused it to use sequential FID numbering (the
driver when seeing more than once a column that is the FID column assumes that
some cross join is done, and thus that FID are unreliable)
A simpler and more robust (crossing fingers!) approach in that case is
just to use OGR_L_SetAttributeFilter() instead of GDALDatasetExecuteSQL().
Some care must be taken to cancel the filter when removing the subset
filter, or in QgsOgrFeatureIterator when combining with the filter
expression coming from the request, but besides that, this is more
straightforward, and actually solves #20136
For complete support, it requires two GDAL fixes:
- One to avoid feature count to be invalid when using ROLLBACK TO SAVEPOINT
f73ec8cd1d
- Another one to avoid nasty issues, at least on Linux, with the POSIX
advisory locks used by libsqlite that could be invalidated due to how GDAL
could open files behind the back of libsqlite. The consequence of this
could be the deletion of -wal and -shm files, which caused issues in QGIS
(non working iterators when the edit is finished, and later edits in the
same session not working). Those issues could appear for example if doing
ogrinfo on the .gpkg opened by QGIS, or if opening two QGIS session on the
.gpkg
Both fixes are queued for GDAL 2.3.1
- Introduce an approximate feature count for GeoPackage in case there are
at least 100,000 rows in a table.
- Add a super fast implementation of GetExtent() for GeoPackage when there
is an RTree
- Do not require feature count when enumerating layers from the browser
I wanted to add the test for gpkg subsetstring even if
it was not bugged, while testing that, I hit an assert
in Qt core that pointed me to double unlocked locks.
GeoPackage/SQLite layers
Previously this capability was only exposed for shapefiles,
but was available in the spatialite provider. We don't use that
for GeoPackages, so I've ported the functionality across to
the OGR provider for these data sources.
Includes unit tests
... to error and data corruption
Fixes#16935 for GPKG (OGR), still needs to check spatialite
Thanks to Matthias Kuhn for pointing me into the right direction
Currently each time you instanciate a QgsOgrProvider layer, a GDAL dataset is
created. In the case of GeoPackage, this means a SQLite connection and a file
handle. As GDAL enables Spatialite function on GeoPackage connections, we are
bound to Spatialite limits, and Spatialite has a hard limit on a maximum of
64 simultaneous connections. Thus we cannot open more than 64 layers of the
same GeoPackage.
This commits enables sharing of the same GDALDataset object among several
QgsOgrProvider object. Care is made to reuse a GDALDataset object only if the
QgsOgrProvider do not point to the same layer. Mutexes are also taken to
allow safe instanciation and use of QgsOgrProvider objects from multiple
threads (but a same QgsOgrProvider should not be used by more than one thread
at a time)
Since the majority of users of this class will be exporting
an existing map layer to a data provider, the QgsVectorLayerImport
name is misleading and suggests that this class is designed
just to bring layers "into" QGIS.
Explicitly naming the class "Exporter" should help API users
discover this class.
Also cleanup API and improve docs
Motivations:
- consistency - we generally use expanded names, and this also
matches Qt API which uses Database instead of Db
- avoids unpredictable capitalization throughout API (mix of "Db"
and "DB")
When moving or deleting a geometry that previously touched the layer extent,
the layer extent was never shrinked.
This fix requires GDAL 2.1.2 or above as well.
Fixes#15273
Concurrent read and write can lock a GeoPackage database given
the default journaling mode of SQLite (delete). Use WAL when
possible to avoid that.
Fixes#15351
