Skip to content

Commit

Permalink
Merge pull request OSGeo#11059 from rouault/gpkg_create_duplicate_fie…
Browse files Browse the repository at this point in the history
…ld_names

GPKG: prevent from creating field with same name or with the name of the geometry field
  • Loading branch information
rouault authored Oct 22, 2024
2 parents 442edd5 + 35d3cb9 commit 350b410
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 0 deletions.
46 changes: 46 additions & 0 deletions autotest/ogr/ogr_gpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -10494,3 +10494,49 @@ def test_gpkg_rename_hidden_table(tmp_vsimem):
gdal.VSIFCloseL(f)

assert "hidden_foo_table" not in content


###############################################################################
# Test creating duplicate field names


@gdaltest.enable_exceptions()
def test_gpkg_create_duplicate_field_names(tmp_vsimem):

filename = str(tmp_vsimem / "test_gpkg_create_duplicate_field_names.gpkg")
with ogr.GetDriverByName("GPKG").CreateDataSource(filename) as ds:
lyr = ds.CreateLayer("test")
lyr.CreateField(ogr.FieldDefn("foo"))
with pytest.raises(
Exception, match="A field with the same name already exists"
):
lyr.CreateField(ogr.FieldDefn("foo"))
assert lyr.GetLayerDefn().GetFieldCount() == 1
with pytest.raises(
Exception, match="A field with the same name already exists"
):
lyr.CreateField(ogr.FieldDefn("FOO"))
assert lyr.GetLayerDefn().GetFieldCount() == 1
with pytest.raises(
Exception, match="It has the same name as the geometry field"
):
lyr.CreateField(ogr.FieldDefn("geom"))
assert lyr.GetLayerDefn().GetFieldCount() == 1


###############################################################################
# Test creating more than 2000 fields


@gdaltest.enable_exceptions()
def test_gpkg_create_more_than_2000_fields(tmp_vsimem):

filename = str(tmp_vsimem / "test_gpkg_create_more_than_2000_fields.gpkg")
with ogr.GetDriverByName("GPKG").CreateDataSource(filename) as ds:
lyr = ds.CreateLayer("test")

for i in range(2000 - 2):
lyr.CreateField(ogr.FieldDefn(f"foo{i}"))
with pytest.raises(Exception, match="Limit of 2000 columns reached"):
lyr.CreateField(ogr.FieldDefn("foo"))
assert lyr.GetLayerDefn().GetFieldCount() == 2000 - 2
31 changes: 31 additions & 0 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,24 @@ OGRErr OGRGeoPackageTableLayer::CreateField(const OGRFieldDefn *poField,
GDALGeoPackageDataset::LaunderName(oFieldDefn.GetNameRef())
.c_str());

if (m_poFeatureDefn->GetFieldIndex(oFieldDefn.GetNameRef()) >= 0)
{
CPLError(CE_Failure, CPLE_AppDefined,
"Cannot create field %s. "
"A field with the same name already exists.",
oFieldDefn.GetNameRef());
return OGRERR_FAILURE;
}

if (m_poFeatureDefn->GetGeomFieldIndex(oFieldDefn.GetNameRef()) >= 0)
{
CPLError(CE_Failure, CPLE_AppDefined,
"Cannot create field %s. "
"It has the same name as the geometry field.",
oFieldDefn.GetNameRef());
return OGRERR_FAILURE;
}

if (m_pszFidColumn != nullptr &&
EQUAL(oFieldDefn.GetNameRef(), m_pszFidColumn) &&
poField->GetType() != OFTInteger &&
Expand All @@ -1742,6 +1760,19 @@ OGRErr OGRGeoPackageTableLayer::CreateField(const OGRFieldDefn *poField,
return OGRERR_FAILURE;
}

const int nMaxColumns =
sqlite3_limit(m_poDS->GetDB(), SQLITE_LIMIT_COLUMN, -1);
// + 1 for the FID column
if (m_poFeatureDefn->GetFieldCount() +
m_poFeatureDefn->GetGeomFieldCount() + 1 >=
nMaxColumns)
{
CPLError(CE_Failure, CPLE_AppDefined,
"Cannot add field %s. Limit of %d columns reached",
oFieldDefn.GetNameRef(), nMaxColumns);
return OGRERR_FAILURE;
}

if (!m_bDeferredCreation)
{
CPLString osCommand;
Expand Down

0 comments on commit 350b410

Please sign in to comment.