Skip to content

Commit

Permalink
GPKG: fix SetFeature()/UpdateFeature()/DeleteFeature() on views with …
Browse files Browse the repository at this point in the history
…INSTEAD OF triggers (fixes OSGeo#8707)
  • Loading branch information
rouault committed Nov 14, 2023
1 parent 8e42f75 commit c4f83c3
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 5 deletions.
33 changes: 31 additions & 2 deletions autotest/ogr/ogr_gpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -6460,6 +6460,7 @@ def test_ogr_gpkg_views(tmp_vsimem, tmp_path):
filename = tmp_vsimem / "test_ogr_gpkg_views.gpkg"
ds = gdaltest.gpkg_dr.CreateDataSource(filename)
lyr = ds.CreateLayer("foo", geom_type=ogr.wkbPoint)
lyr.CreateField(ogr.FieldDefn("str_field"))
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(ogr.CreateGeometryFromWkt("POINT(0 0)"))
lyr.CreateFeature(f)
Expand All @@ -6477,7 +6478,7 @@ def test_ogr_gpkg_views(tmp_vsimem, tmp_path):
"INSERT INTO gpkg_geometry_columns (table_name, column_name, geometry_type_name, srs_id, z, m) values ('geom_view', 'my_geom', 'POINT', 0, 0, 0)"
)

ds.ExecuteSQL("CREATE VIEW attr_view AS SELECT fid AS my_fid FROM foo")
ds.ExecuteSQL("CREATE VIEW attr_view AS SELECT fid AS my_fid, str_field FROM foo")
ds.ExecuteSQL(
"INSERT INTO gpkg_contents (table_name, identifier, data_type) VALUES ( 'attr_view', 'attr_view', 'attributes' )"
)
Expand All @@ -6486,7 +6487,7 @@ def test_ogr_gpkg_views(tmp_vsimem, tmp_path):

assert validate(filename, tmpdir=tmp_path), "validation failed"

ds = ogr.Open(filename)
ds = ogr.Open(filename, update=1)
assert ds.GetLayerCount() == 3

lyr = ds.GetLayerByName("geom_view")
Expand All @@ -6495,6 +6496,34 @@ def test_ogr_gpkg_views(tmp_vsimem, tmp_path):
lyr = ds.GetLayerByName("attr_view")
assert lyr.GetGeomType() == ogr.wkbNone

f = lyr.GetFeature(1)
f["str_field"] = "bar"
with gdal.quiet_errors():
assert lyr.SetFeature(f) == ogr.OGRERR_FAILURE

ds.ExecuteSQL(
"CREATE TRIGGER attr_view_str_field_chng INSTEAD OF UPDATE OF str_field ON attr_view BEGIN UPDATE foo SET str_field=NEW.str_field WHERE fid=NEW.my_fid; END;"
)

assert lyr.SetFeature(f) == ogr.OGRERR_NONE
assert lyr.UpdateFeature(f, [0], [], False) == ogr.OGRERR_NONE

f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(100)
f["str_field"] = "bar"
assert lyr.SetFeature(f) == ogr.OGRERR_NON_EXISTING_FEATURE
assert lyr.UpdateFeature(f, [0], [], False) == ogr.OGRERR_NON_EXISTING_FEATURE

with gdal.quiet_errors():
assert lyr.DeleteFeature(1) == ogr.OGRERR_FAILURE

ds.ExecuteSQL(
"CREATE TRIGGER attr_view_delete INSTEAD OF DELETE ON attr_view BEGIN DELETE FROM foo WHERE fid=OLD.my_fid; END;"
)

assert lyr.DeleteFeature(1) == ogr.OGRERR_NONE
assert lyr.DeleteFeature(100) == ogr.OGRERR_NON_EXISTING_FEATURE

ds = None


Expand Down
48 changes: 45 additions & 3 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2977,6 +2977,13 @@ OGRErr OGRGeoPackageTableLayer::ISetFeature(OGRFeature *poFeature)
if (!RunDeferredSpatialIndexUpdate())
return OGRERR_FAILURE;

const sqlite3_int64 nTotalChangesBefore =
#if SQLITE_VERSION_NUMBER >= 3036000L
sqlite3_total_changes64(m_poDS->GetDB());
#else
sqlite3_total_changes(m_poDS->GetDB());
#endif

CheckGeometryType(poFeature);

if (!m_osUpdateStatementSQL.empty())
Expand Down Expand Up @@ -3031,8 +3038,15 @@ OGRErr OGRGeoPackageTableLayer::ISetFeature(OGRFeature *poFeature)
sqlite3_reset(m_poUpdateStatement);
sqlite3_clear_bindings(m_poUpdateStatement);

const sqlite3_int64 nTotalChangesAfter =
#if SQLITE_VERSION_NUMBER >= 3036000L
sqlite3_total_changes64(m_poDS->GetDB());
#else
sqlite3_total_changes(m_poDS->GetDB());
#endif

/* Only update the envelope if we changed something */
OGRErr eErr = (sqlite3_changes(m_poDS->GetDB()) > 0)
OGRErr eErr = nTotalChangesAfter != nTotalChangesBefore
? OGRERR_NONE
: OGRERR_NON_EXISTING_FEATURE;
if (eErr == OGRERR_NONE)
Expand Down Expand Up @@ -3230,6 +3244,13 @@ OGRErr OGRGeoPackageTableLayer::IUpdateFeature(
return OGRERR_FAILURE;
}

const sqlite3_int64 nTotalChangesBefore =
#if SQLITE_VERSION_NUMBER >= 3036000L
sqlite3_total_changes64(m_poDS->GetDB());
#else
sqlite3_total_changes(m_poDS->GetDB());
#endif

/* From here execute the statement and check errors */
int err = sqlite3_step(m_poUpdateStatement);
if (!(err == SQLITE_OK || err == SQLITE_DONE))
Expand All @@ -3244,8 +3265,15 @@ OGRErr OGRGeoPackageTableLayer::IUpdateFeature(
sqlite3_reset(m_poUpdateStatement);
sqlite3_clear_bindings(m_poUpdateStatement);

const sqlite3_int64 nTotalChangesAfter =
#if SQLITE_VERSION_NUMBER >= 3036000L
sqlite3_total_changes64(m_poDS->GetDB());
#else
sqlite3_total_changes(m_poDS->GetDB());
#endif

/* Only update the envelope if we changed something */
OGRErr eErr = (sqlite3_changes(m_poDS->GetDB()) > 0)
OGRErr eErr = nTotalChangesAfter != nTotalChangesBefore
? OGRERR_NONE
: OGRERR_NON_EXISTING_FEATURE;
if (eErr == OGRERR_NONE)
Expand Down Expand Up @@ -3570,10 +3598,24 @@ OGRErr OGRGeoPackageTableLayer::DeleteFeature(GIntBig nFID)
SQLEscapeName(m_pszTableName).c_str(),
SQLEscapeName(m_pszFidColumn).c_str(), nFID);

const sqlite3_int64 nTotalChangesBefore =
#if SQLITE_VERSION_NUMBER >= 3036000L
sqlite3_total_changes64(m_poDS->GetDB());
#else
sqlite3_total_changes(m_poDS->GetDB());
#endif

OGRErr eErr = SQLCommand(m_poDS->GetDB(), soSQL.c_str());
if (eErr == OGRERR_NONE)
{
eErr = (sqlite3_changes(m_poDS->GetDB()) > 0)
const sqlite3_int64 nTotalChangesAfter =
#if SQLITE_VERSION_NUMBER >= 3036000L
sqlite3_total_changes64(m_poDS->GetDB());
#else
sqlite3_total_changes(m_poDS->GetDB());
#endif

eErr = nTotalChangesAfter != nTotalChangesBefore
? OGRERR_NONE
: OGRERR_NON_EXISTING_FEATURE;

Expand Down

0 comments on commit c4f83c3

Please sign in to comment.