Skip to content

Commit

Permalink
Merge pull request OSGeo#3741 from rouault/gpkg_tidy
Browse files Browse the repository at this point in the history 
GPKG: fix a few comments, and improve a bit testing
  • Loading branch information
@rouault
rouault authored Apr 25, 2021
2 parents ba2bc11 + 03e8ba2 commit b2a7857
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 22 deletions.
45 changes: 27 additions & 18 deletions autotest/ogr/ogr_gpkg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1920,9 +1920,6 @@ def test_ogr_gpkg_23():

def test_ogr_gpkg_unique():

if gdaltest.is_travis_branch('trusty_32bit') or gdaltest.is_travis_branch('trusty_clang'):
pytest.skip('gcc too old')

ds = ogr.GetDriverByName('GPKG').CreateDataSource('/vsimem/ogr_gpkg_unique.gpkg')
lyr = ds.CreateLayer('test', geom_type=ogr.wkbNone)

Expand All @@ -1949,6 +1946,16 @@ def test_ogr_gpkg_unique():
fldDef = layerDefinition.GetFieldDefn(2)
assert fldDef.IsUnique()

f = ogr.Feature(layerDefinition)
assert lyr.CreateFeature(f) == ogr.OGRERR_NONE
f = None

# Test adding columns after "crystallization"
field_defn = ogr.FieldDefn('field_unique_failure', ogr.OFTString)
field_defn.SetUnique(1)
# Not allowed by sqlite3. Could potentially be improved
with gdaltest.error_handler():
assert lyr.CreateField(field_defn) == ogr.OGRERR_FAILURE

# Create another layer from SQL to test quoting of fields
# and indexes
Expand Down Expand Up @@ -4223,19 +4230,19 @@ def test_ogr_gpkg_fixup_wrong_rtree_trigger():
ds.CreateLayer('test')
ds.CreateLayer('test2')
ds = None
with gdaltest.error_handler():
ds = ogr.Open(filename, update = 1)
# inject wrong trigger on purpose with the wrong 'OF "geometry" ' part
ds.ExecuteSQL('DROP TRIGGER rtree_test_geometry_update3')
wrong_trigger = 'CREATE TRIGGER "rtree_test_geometry_update3" AFTER UPDATE OF "geometry" ON "test" WHEN OLD."fid" != NEW."fid" AND (NEW."geometry" NOTNULL AND NOT ST_IsEmpty(NEW."geometry")) BEGIN DELETE FROM "rtree_test_geometry" WHERE id = OLD."fid"; INSERT OR REPLACE INTO "rtree_test_geometry" VALUES (NEW."fid",ST_MinX(NEW."geometry"), ST_MaxX(NEW."geometry"),ST_MinY(NEW."geometry"), ST_MaxY(NEW."geometry")); END'
ds.ExecuteSQL(wrong_trigger)

ds = ogr.Open(filename, update = 1)
# inject wrong trigger on purpose with the wrong 'OF "geometry" ' part
ds.ExecuteSQL('DROP TRIGGER rtree_test_geometry_update3')
wrong_trigger = 'CREATE TRIGGER "rtree_test_geometry_update3" AFTER UPDATE OF "geometry" ON "test" WHEN OLD."fid" != NEW."fid" AND (NEW."geometry" NOTNULL AND NOT ST_IsEmpty(NEW."geometry")) BEGIN DELETE FROM "rtree_test_geometry" WHERE id = OLD."fid"; INSERT OR REPLACE INTO "rtree_test_geometry" VALUES (NEW."fid",ST_MinX(NEW."geometry"), ST_MaxX(NEW."geometry"),ST_MinY(NEW."geometry"), ST_MaxY(NEW."geometry")); END'
ds.ExecuteSQL(wrong_trigger)

ds.ExecuteSQL('DROP TRIGGER rtree_test2_geometry_update3')
# Test another potential variant (although not generated by OGR)
wrong_trigger2 = 'CREATE TRIGGER "rtree_test2_geometry_update3" AFTER UPDATE OF geometry ON test2 WHEN OLD."fid" != NEW."fid" AND (NEW."geometry" NOTNULL AND NOT ST_IsEmpty(NEW."geometry")) BEGIN DELETE FROM "rtree_test_geometry" WHERE id = OLD."fid"; INSERT OR REPLACE INTO "rtree_test_geometry" VALUES (NEW."fid",ST_MinX(NEW."geometry"), ST_MaxX(NEW."geometry"),ST_MinY(NEW."geometry"), ST_MaxY(NEW."geometry")); END'
ds.ExecuteSQL(wrong_trigger2)
ds.ExecuteSQL('DROP TRIGGER rtree_test2_geometry_update3')
# Test another potential variant (although not generated by OGR)
wrong_trigger2 = 'CREATE TRIGGER "rtree_test2_geometry_update3" AFTER UPDATE OF geometry ON test2 WHEN OLD."fid" != NEW."fid" AND (NEW."geometry" NOTNULL AND NOT ST_IsEmpty(NEW."geometry")) BEGIN DELETE FROM "rtree_test_geometry" WHERE id = OLD."fid"; INSERT OR REPLACE INTO "rtree_test_geometry" VALUES (NEW."fid",ST_MinX(NEW."geometry"), ST_MaxX(NEW."geometry"),ST_MinY(NEW."geometry"), ST_MaxY(NEW."geometry")); END'
ds.ExecuteSQL(wrong_trigger2)

ds = None
ds = None

# Open in read-only mode
ds = ogr.Open(filename)
Expand Down Expand Up @@ -4321,7 +4328,7 @@ def test_abort_sql():
ds = ogr.Open(filename)

def abortAfterDelay():
print("Aborting SQL...")
#print("Aborting SQL...")
assert ds.AbortSQL() == ogr.OGRERR_NONE

t = threading.Timer(0.5, abortAfterDelay)
Expand All @@ -4339,7 +4346,8 @@ def abortAfterDelay():
)
SELECT i FROM r WHERE i = 1;"""

ds.ExecuteSQL(sql)
with gdaltest.error_handler():
ds.ExecuteSQL(sql)

end = time.time()
assert int(end - start) < 1
Expand All @@ -4348,7 +4356,7 @@ def abortAfterDelay():
ds2 = gdal.OpenEx(filename, gdal.OF_VECTOR)

def abortAfterDelay2():
print("Aborting SQL...")
#print("Aborting SQL...")
assert ds2.AbortSQL() == ogr.OGRERR_NONE

t = threading.Timer(0.5, abortAfterDelay2)
Expand All @@ -4357,7 +4365,8 @@ def abortAfterDelay2():
start = time.time()

# Long running query
ds2.ExecuteSQL(sql)
with gdaltest.error_handler():
ds2.ExecuteSQL(sql)

end = time.time()
assert int(end - start) < 1
Expand Down
15 changes: 11 additions & 4 deletions gdal/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,9 @@ OGRErr OGRGeoPackageTableLayer::CreateField( OGRFieldDefn *poField,
{
CPLString osCommand;

// ADD COLUMN has several restrictions
// See https://www.sqlite.org/lang_altertable.html#altertabaddcol

osCommand.Printf("ALTER TABLE \"%s\" ADD COLUMN \"%s\" %s",
SQLEscapeName(m_pszTableName).c_str(),
SQLEscapeName(poField->GetNameRef()).c_str(),
Expand All @@ -1345,7 +1348,12 @@ OGRErr OGRGeoPackageTableLayer::CreateField( OGRFieldDefn *poField,
if( !poField->IsNullable() )
osCommand += " NOT NULL";
if( poField->IsUnique() )
{
// this will fail when SQLCommand() is run, as it is not allowed
// by SQLite. This is a bit of an artificial restriction.
// We could override it by rewriting the table.
osCommand += " UNIQUE";
}
if( poField->GetDefault() != nullptr && !poField->IsDefaultDriverSpecific() )
{
osCommand += " DEFAULT ";
Expand All @@ -1369,15 +1377,14 @@ OGRErr OGRGeoPackageTableLayer::CreateField( OGRFieldDefn *poField,
}
else
{
// This could fail if it is CURRENT_TIMESTAMP, etc.
osCommand += poField->GetDefault();
}
}
else if( !poField->IsNullable() )
{
// This is kind of dumb, but SQLite mandates a DEFAULT value
// when adding a NOT NULL column in an ALTER TABLE ADD COLUMN
// statement, which defeats the purpose of NOT NULL,
// whereas it doesn't in CREATE TABLE
// SQLite mandates a DEFAULT value when adding a NOT NULL column in
// an ALTER TABLE ADD COLUMN.
osCommand += " DEFAULT ''";
}

Expand Down

0 comments on commit b2a7857

Please sign in to comment.