Fix GPKG and Spatialite UNIQUE and NOT NULL constraints detection #36802
Conversation
elpaso
added
Data Provider
| @@ -1099,6 +1103,22 @@ void QgsOgrProvider::loadFields() | |||
| mFirstFieldIsFid = !fidColumn.isEmpty() && | |||
| fdef.GetFieldIndex( fidColumn ) < 0; | |||
|
|
|||
| // This is a temporary solution until GDAL Unique support is available | |||
| QSet<QString> uniqueFieldNames; | |||
There was a problem hiding this comment.
are you considering to have a GDAL >= 3.2 path that uses the GDAL API (in this PR or a follow-up once OSGeo/gdal#2622 is merged) ?
There was a problem hiding this comment.
@rouault yes, that was the idea.
But I'm now thinking to remove the OGR part from this PR leaving spatialite (because it fixes another bug about NOT NULL) and wait for GDAL API to be ready, there is failing test about number of connections, https://travis-ci.org/github/qgis/QGIS/builds/692197527#L2175
I don't get exactly why it happens, because the solution in the patch does actually open an additional connection but it should be closed immediately thanks to the sqlite3_database_unique_ptr.
Anyway I've already spent faaaaar too much time on this issue and I don't think I can spend more.
I also tried a different implementation where I pass the sqlite3 handle to the uniqueValues method (and make it static) but it seems there is no way to retrieve it from the OGR layer.
There was a problem hiding this comment.
yes the failure is mysterious... And there is indeed no way to get the sqlite3 handle from a GDALDataset.
There was a problem hiding this comment.
@rouault I've checked and the sqlite3_database_unique_ptr deleter actually called sqlite3_close_v2, how do you feel about changing the test expected value?
I can add a note that it is a temporary solution until we use the GDAL implementation.
I'd really like to avoid putting yet another sql parser inside the OGR provider.
There was a problem hiding this comment.
how do you feel about changing the test expected value?
I can add a note that it is a temporary solution until we use the GDAL implementation.
yeah, we can do that. Odd though
elpaso
force-pushed
the
bugfix-36468-unique-tests
branch
2 times, most recently
from
6611d63
to
a00195f
Compare
Fixes qgis#36468 and un reported NOT NULL detection failure in spatialite.
This is a temporary solution for OGR because we expect to use the native GDAL implementation in GDAL 3.2. For now a new method QgsSqliteUtils::uniqueFields used by spatialite and OGR/GPKG is used to detect UNIQUE constraints on single fields.
elpaso
force-pushed
the
bugfix-36468-unique-tests
branch
from
a00195f
to
1bdaf15
Compare
Fixes #36468 and unreported NOT NULL
detection failure in spatialite.
Also adds tests for both in the main testsuite for providers.
Unique detection is being implemented in GDAL too with the same logic and more tests, so this is meant to be partly (for OGR) a temporary solution until GDAL implementation is ready, for spatialite it will remain a valid patch because the native provider does not use GDAL.