diff --git a/autotest/ogr/ogr_gpkg.py b/autotest/ogr/ogr_gpkg.py index c23424e48786..1d8b00fe595d 100755 --- a/autotest/ogr/ogr_gpkg.py +++ b/autotest/ogr/ogr_gpkg.py @@ -2569,6 +2569,18 @@ def test_ogr_gpkg_34(): assert gdal.GetLastErrorMsg() != '' gdal.ErrorReset() ds.ExecuteSQL('ALTER TABLE "weird\'layer""name" RENAME TO "weird2\'layer""name"') + ds = None + + ds = ogr.Open(dbname, update=1) + ds.ExecuteSQL('ALTER TABLE "weird2\'layer""name" RENAME COLUMN "foo" TO "bar"') + assert gdal.GetLastErrorMsg() == '' + lyr = ds.GetLayerByName(new_layer_name) + assert lyr.GetLayerDefn().GetFieldIndex('bar') >= 0 + + ds.ExecuteSQL('ALTER TABLE "weird2\'layer""name" DROP COLUMN "bar"') + assert gdal.GetLastErrorMsg() == '' + assert lyr.GetLayerDefn().GetFieldIndex('bar') < 0 + ds.ExecuteSQL('VACUUM') ds = None @@ -2832,8 +2844,26 @@ def test_ogr_gpkg_36(): new_field_defn = ogr.FieldDefn('bar', ogr.OFTReal) new_field_defn.SetSubType(ogr.OFSTFloat32) + new_field_defn.SetWidth(10) + new_field_defn.SetDefault('5') + + # Schema only change + assert lyr.AlterFieldDefn(0, new_field_defn, ogr.ALTER_ALL_FLAG) == 0 + + # Full table rewrite + new_field_defn.SetNullable(False) + assert lyr.AlterFieldDefn(0, new_field_defn, ogr.ALTER_ALL_FLAG) == 0 + + # Full table rewrite + new_field_defn.SetUnique(True) assert lyr.AlterFieldDefn(0, new_field_defn, ogr.ALTER_ALL_FLAG) == 0 + # Violation of not-null constraint + new_field_defn = ogr.FieldDefn('baz', ogr.OFTString) + new_field_defn.SetNullable(False) + with gdaltest.error_handler(): + assert lyr.AlterFieldDefn(1 , new_field_defn, ogr.ALTER_ALL_FLAG) != 0 + lyr.ResetReading() f = lyr.GetNextFeature() if f.GetFID() != 10 or f['bar'] != 10.5 or \ @@ -2842,6 +2872,13 @@ def test_ogr_gpkg_36(): pytest.fail() f = None + # Just change the name, and run it outside an existing transaction + lyr.StartTransaction() + new_field_defn = ogr.FieldDefn('baw2', ogr.OFTString) + assert lyr.AlterFieldDefn(0, new_field_defn, ogr.ALTER_ALL_FLAG) == 0 + lyr.CommitTransaction() + + # Just change the name, and run it under an existing transaction lyr.StartTransaction() new_field_defn = ogr.FieldDefn('baw', ogr.OFTString) assert lyr.AlterFieldDefn(0, new_field_defn, ogr.ALTER_ALL_FLAG) == 0 @@ -2902,7 +2939,9 @@ def test_ogr_gpkg_36(): # Unlink before AlterFieldDefn gdal.Unlink(dbname) with gdaltest.error_handler(): - ret = lyr.AlterFieldDefn(0, ogr.FieldDefn('bar'), ogr.ALTER_ALL_FLAG) + new_field_defn = ogr.FieldDefn('bar') + new_field_defn.SetNullable(False) + ret = lyr.AlterFieldDefn(0, new_field_defn, ogr.ALTER_ALL_FLAG) assert ret != 0 with gdaltest.error_handler(): ds = None diff --git a/autotest/ogr/ogr_rfc35_sqlite.py b/autotest/ogr/ogr_rfc35_sqlite.py index 047f085111f4..216fa26d12b8 100755 --- a/autotest/ogr/ogr_rfc35_sqlite.py +++ b/autotest/ogr/ogr_rfc35_sqlite.py @@ -133,7 +133,7 @@ def CheckFeatures(lyr, field1='foo5', field2='bar10', field3='baz15', field4='ba feat = lyr.GetNextFeature() i = i + 1 - + def CheckColumnOrder(lyr, expected_order): @@ -224,19 +224,27 @@ def test_ogr_rfc35_sqlite_3(): CheckFeatures(lyr, field3='baz25') - fd = ogr.FieldDefn("baz5", ogr.OFTString) + fd = ogr.FieldDefn("baz5_tmp", ogr.OFTString) fd.SetWidth(5) lyr_defn = lyr.GetLayerDefn() lyr.AlterFieldDefn(lyr_defn.GetFieldIndex("baz25"), fd, ogr.ALTER_ALL_FLAG) - CheckFeatures(lyr, field3='baz5') + CheckFeatures(lyr, field3='baz5_tmp') lyr_defn = lyr.GetLayerDefn() - fld_defn = lyr_defn.GetFieldDefn(lyr_defn.GetFieldIndex('baz5')) + fld_defn = lyr_defn.GetFieldDefn(lyr_defn.GetFieldIndex('baz5_tmp')) assert fld_defn.GetWidth() == 5 - CheckFeatures(lyr, field3='baz5') + CheckFeatures(lyr, field3='baz5_tmp') + + # Change only name + fd = ogr.FieldDefn("baz5", ogr.OFTString) + fd.SetWidth(5) + lyr.AlterFieldDefn(lyr_defn.GetFieldIndex("baz5_tmp"), fd, ogr.ALTER_ALL_FLAG) + + fld_defn = lyr_defn.GetFieldDefn(lyr_defn.GetFieldIndex('baz5')) + assert fld_defn.GetWidth() == 5 ############################################################################### # Test AlterFieldDefn() for change of type diff --git a/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h b/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h index 257c6d6b34c5..2935946caedc 100644 --- a/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h +++ b/ogr/ogrsf_frmts/gpkg/ogr_geopackage.h @@ -327,7 +327,6 @@ class GDALGeoPackageDataset final : public OGRSQLiteBaseDataSource, public GDALG private: - OGRErr PragmaCheck(const char * pszPragma, const char * pszExpected, int nRowsExpected); OGRErr SetApplicationAndUserVersionId(); bool ReOpenDB(); bool OpenOrCreateDB( int flags ); diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp index 2a1915f69622..8ea6c252fc29 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp @@ -271,56 +271,6 @@ bool GDALGeoPackageDataset::ReOpenDB() return OpenOrCreateDB(SQLITE_OPEN_READWRITE); } -/* Returns the first row of first column of SQL as integer */ -OGRErr GDALGeoPackageDataset::PragmaCheck( - const char * pszPragma, const char * pszExpected, int nRowsExpected ) -{ - CPLAssert( pszPragma != nullptr ); - CPLAssert( pszExpected != nullptr ); - CPLAssert( nRowsExpected >= 0 ); - - char **papszResult = nullptr; - int nRowCount = 0; - int nColCount = 0; - char *pszErrMsg = nullptr; - - int rc = sqlite3_get_table( - hDB, - CPLSPrintf("PRAGMA %s", pszPragma), - &papszResult, &nRowCount, &nColCount, &pszErrMsg ); - - if( rc != SQLITE_OK ) - { - CPLError( CE_Failure, CPLE_AppDefined, - "Unable to execute PRAGMA %s: %s", pszPragma, - pszErrMsg ? pszErrMsg : "(null)" ); - sqlite3_free( pszErrMsg ); - return OGRERR_FAILURE; - } - - if ( nRowCount != nRowsExpected ) - { - CPLError( CE_Failure, CPLE_AppDefined, - "bad result for PRAGMA %s, got %d rows, expected %d", - pszPragma, nRowCount, nRowsExpected ); - sqlite3_free_table(papszResult); - return OGRERR_FAILURE; - } - - if ( nRowCount > 0 && ! EQUAL(papszResult[1], pszExpected) ) - { - CPLError( CE_Failure, CPLE_AppDefined, - "invalid %s (expected '%s', got '%s')", - pszPragma, pszExpected, papszResult[1]); - sqlite3_free_table(papszResult); - return OGRERR_FAILURE; - } - - sqlite3_free_table(papszResult); - - return OGRERR_NONE; -} - static OGRErr GDALGPKGImportFromEPSG(OGRSpatialReference *poSpatialRef, int nEPSGCode) { @@ -5959,7 +5909,13 @@ OGRLayer * GDALGeoPackageDataset::ExecuteSQL( const char *pszSQLCommand, } /* -------------------------------------------------------------------- */ -/* Intercept ALTER TABLE ... RENAME TO */ +/* Intercept ALTER TABLE src_table RENAME TO dst_table */ +/* and ALTER TABLE table RENAME COLUMN src_name TO dst_name */ +/* and ALTER TABLE table DROP COLUMN col_name */ +/* */ +/* We do this because SQLite mechanisms can't deal with updating */ +/* literal values in gpkg_ tables that refer to table and column */ +/* names. */ /* -------------------------------------------------------------------- */ if( STARTS_WITH_CI(osSQLCommand, "ALTER TABLE ") ) { @@ -5971,8 +5927,8 @@ OGRLayer * GDALGeoPackageDataset::ExecuteSQL( const char *pszSQLCommand, const char* pszSrcTableName = papszTokens[2]; const char* pszDstTableName = papszTokens[5]; OGRGeoPackageTableLayer* poSrcLayer = - (OGRGeoPackageTableLayer*)GetLayerByName( - SQLUnescape(pszSrcTableName)); + dynamic_cast(GetLayerByName( + SQLUnescape(pszSrcTableName))); if( poSrcLayer ) { poSrcLayer->RenameTo( SQLUnescape(pszDstTableName) ); @@ -5980,6 +5936,53 @@ OGRLayer * GDALGeoPackageDataset::ExecuteSQL( const char *pszSQLCommand, return nullptr; } } + /* ALTER TABLE table RENAME COLUMN src_name TO dst_name */ + else if( CSLCount(papszTokens) == 8 && + EQUAL(papszTokens[3], "RENAME") && + EQUAL(papszTokens[4], "COLUMN") && + EQUAL(papszTokens[6], "TO") ) + { + const char* pszTableName = papszTokens[2]; + const char* pszSrcColumn = papszTokens[5]; + const char* pszDstColumn = papszTokens[7]; + OGRGeoPackageTableLayer* poLayer = + dynamic_cast(GetLayerByName( + SQLUnescape(pszTableName))); + if( poLayer ) + { + int nSrcFieldIdx = poLayer->GetLayerDefn()->GetFieldIndex(SQLUnescape(pszSrcColumn)); + if( nSrcFieldIdx >= 0 ) + { + // OFTString or any type will do as we just alter the name + // so it will be ignored. + OGRFieldDefn oFieldDefn(SQLUnescape(pszDstColumn), OFTString); + poLayer->AlterFieldDefn(nSrcFieldIdx, &oFieldDefn, ALTER_NAME_FLAG); + CSLDestroy(papszTokens); + return nullptr; + } + } + } + /* ALTER TABLE table DROP COLUMN col_name */ + else if( CSLCount(papszTokens) == 6 && + EQUAL(papszTokens[3], "DROP") && + EQUAL(papszTokens[4], "COLUMN") ) + { + const char* pszTableName = papszTokens[2]; + const char* pszColumName = papszTokens[5]; + OGRGeoPackageTableLayer* poLayer = + dynamic_cast(GetLayerByName( + SQLUnescape(pszTableName))); + if( poLayer ) + { + int nFieldIdx = poLayer->GetLayerDefn()->GetFieldIndex(SQLUnescape(pszColumName)); + if( nFieldIdx >= 0 ) + { + poLayer->DeleteField(nFieldIdx); + CSLDestroy(papszTokens); + return nullptr; + } + } + } CSLDestroy(papszTokens); } diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp index 97a2cc2753f9..7307ad79140e 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp @@ -4560,7 +4560,27 @@ OGRErr OGRGeoPackageTableLayer::DeleteField( int iFieldToDelete ) if( !RunDeferredSpatialIndexUpdate() ) return OGRERR_FAILURE; + const char* pszFieldName = + m_poFeatureDefn->GetFieldDefn(iFieldToDelete)->GetNameRef(); + +/* -------------------------------------------------------------------- */ +/* Drop any iterator since we change the DB structure */ /* -------------------------------------------------------------------- */ + m_poDS->ResetReadingAllLayers(); + + if( m_poDS->SoftStartTransaction() != OGRERR_NONE ) + return OGRERR_FAILURE; + + // ALTER TABLE ... DROP COLUMN ... was first implemented in 3.35.0 but + // there was bug fixes related to it until 3.35.5 +#if SQLITE_VERSION_NUMBER >= 3035005L + OGRErr eErr = SQLCommand( m_poDS->GetDB(), + CPLString().Printf("ALTER TABLE \"%s\" DROP COLUMN \"%s\"", + SQLEscapeName(m_pszTableName).c_str(), + SQLEscapeName(pszFieldName).c_str()).c_str() ); +#else +/* -------------------------------------------------------------------- */ +/* Recreate table in a transaction */ /* Build list of old fields, and the list of new fields. */ /* -------------------------------------------------------------------- */ std::vector apoFields; @@ -4576,21 +4596,8 @@ OGRErr OGRGeoPackageTableLayer::DeleteField( int iFieldToDelete ) CPLString osFieldListForSelect( BuildSelectFieldList(apoFields) ); CPLString osColumnsForCreate( GetColumnsOfCreateTable(apoFields) ); -/* -------------------------------------------------------------------- */ -/* Drop any iterator since we change the DB structure */ -/* -------------------------------------------------------------------- */ - m_poDS->ResetReadingAllLayers(); - -/* -------------------------------------------------------------------- */ -/* Recreate table in a transaction */ -/* -------------------------------------------------------------------- */ - if( m_poDS->SoftStartTransaction() != OGRERR_NONE ) - return OGRERR_FAILURE; - OGRErr eErr = RecreateTable(osColumnsForCreate, osFieldListForSelect); - - const char* pszFieldName = - m_poFeatureDefn->GetFieldDefn(iFieldToDelete)->GetNameRef(); +#endif /* -------------------------------------------------------------------- */ /* Update gpkg_extensions if needed. */ @@ -4671,10 +4678,13 @@ OGRErr OGRGeoPackageTableLayer::DeleteField( int iFieldToDelete ) } /* -------------------------------------------------------------------- */ -/* Check foreign key integrity. */ +/* Check foreign key integrity if enforcement of foreign keys */ +/* constraint is enabled. */ /* -------------------------------------------------------------------- */ - if( eErr == OGRERR_NONE ) + if( eErr == OGRERR_NONE && + SQLGetInteger(m_poDS->GetDB(), "PRAGMA foreign_keys", nullptr) ) { + CPLDebug("GPKG", "Running PRAGMA foreign_key_check"); eErr = m_poDS->PragmaCheck("foreign_key_check", "", 0); } @@ -4731,8 +4741,8 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, /* Check that the new column name is not a duplicate. */ /* -------------------------------------------------------------------- */ - const CPLString osOldColName( - m_poFeatureDefn->GetFieldDefn(iFieldToAlter)->GetNameRef() ); + OGRFieldDefn* poFieldDefnToAlter = m_poFeatureDefn->GetFieldDefn(iFieldToAlter); + const CPLString osOldColName( poFieldDefnToAlter->GetNameRef() ); const CPLString osNewColName( (nFlagsIn & ALTER_NAME_FLAG) ? CPLString(poNewFieldDefn->GetNameRef()) : osOldColName ); @@ -4757,38 +4767,65 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, } /* -------------------------------------------------------------------- */ -/* Build list of old fields, and the list of new fields. */ +/* Build the modified field definition from the flags. */ /* -------------------------------------------------------------------- */ - OGRFieldDefn oTmpFieldDefn(m_poFeatureDefn->GetFieldDefn(iFieldToAlter)); - if( (nFlagsIn & ALTER_NAME_FLAG) ) + OGRFieldDefn oTmpFieldDefn(poFieldDefnToAlter); + bool bUseRewriteSchemaMethod = ( m_poDS->nSoftTransactionLevel == 0 ); + int nActualFlags = 0; + if( bRenameCol ) + { + nActualFlags |= ALTER_NAME_FLAG; oTmpFieldDefn.SetName(poNewFieldDefn->GetNameRef()); - if( (nFlagsIn & ALTER_TYPE_FLAG) ) + } + if( (nFlagsIn & ALTER_TYPE_FLAG) != 0 && + (poFieldDefnToAlter->GetType() != poNewFieldDefn->GetType() || + poFieldDefnToAlter->GetSubType() != poNewFieldDefn->GetSubType()) ) { + nActualFlags |= ALTER_TYPE_FLAG; oTmpFieldDefn.SetSubType(OFSTNone); oTmpFieldDefn.SetType(poNewFieldDefn->GetType()); oTmpFieldDefn.SetSubType(poNewFieldDefn->GetSubType()); } - if (nFlagsIn & ALTER_WIDTH_PRECISION_FLAG) + if ( (nFlagsIn & ALTER_WIDTH_PRECISION_FLAG) != 0 && + (poFieldDefnToAlter->GetWidth() != poNewFieldDefn->GetWidth() || + poFieldDefnToAlter->GetPrecision() != poNewFieldDefn->GetPrecision()) ) { + nActualFlags |= ALTER_WIDTH_PRECISION_FLAG; oTmpFieldDefn.SetWidth(poNewFieldDefn->GetWidth()); oTmpFieldDefn.SetPrecision(poNewFieldDefn->GetPrecision()); } - if( (nFlagsIn & ALTER_NULLABLE_FLAG) ) + if( (nFlagsIn & ALTER_NULLABLE_FLAG) != 0 && + poFieldDefnToAlter->IsNullable() != poNewFieldDefn->IsNullable() ) { + nActualFlags |= ALTER_NULLABLE_FLAG; + bUseRewriteSchemaMethod = false; oTmpFieldDefn.SetNullable(poNewFieldDefn->IsNullable()); } - if( (nFlagsIn & ALTER_DEFAULT_FLAG) ) + if( (nFlagsIn & ALTER_DEFAULT_FLAG) != 0 && + !( (poFieldDefnToAlter->GetDefault() == nullptr && poNewFieldDefn->GetDefault() == nullptr) || + (poFieldDefnToAlter->GetDefault() != nullptr && poNewFieldDefn->GetDefault() != nullptr && + strcmp(poFieldDefnToAlter->GetDefault(), poNewFieldDefn->GetDefault()) == 0) ) ) { + nActualFlags |= ALTER_DEFAULT_FLAG; oTmpFieldDefn.SetDefault(poNewFieldDefn->GetDefault()); } - if( (nFlagsIn & ALTER_UNIQUE_FLAG) ) + if( (nFlagsIn & ALTER_UNIQUE_FLAG) != 0 && + poFieldDefnToAlter->IsUnique() != poNewFieldDefn->IsUnique() ) { - oTmpFieldDefn.SetUnique( poNewFieldDefn->IsUnique()); + nActualFlags |= ALTER_UNIQUE_FLAG; + bUseRewriteSchemaMethod = false; + oTmpFieldDefn.SetUnique( poNewFieldDefn->IsUnique()); } - if( (nFlagsIn & ALTER_DOMAIN_FLAG) ) + if( (nFlagsIn & ALTER_DOMAIN_FLAG) != 0 && + poFieldDefnToAlter->GetDomainName() != poNewFieldDefn->GetDomainName() ) { - oTmpFieldDefn.SetDomainName( poNewFieldDefn->GetDomainName()); + nActualFlags |= ALTER_DOMAIN_FLAG; + oTmpFieldDefn.SetDomainName( poNewFieldDefn->GetDomainName()); } + +/* -------------------------------------------------------------------- */ +/* Build list of old fields, and the list of new fields. */ +/* -------------------------------------------------------------------- */ std::vector apoFields; std::vector apoFieldsOld; for( int iField = 0; iField < m_poFeatureDefn->GetFieldCount(); iField++ ) @@ -4813,7 +4850,15 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, /* -------------------------------------------------------------------- */ m_poDS->ResetReadingAllLayers(); - const bool bUseFastMethod = ( m_poDS->nSoftTransactionLevel == 0 ); + // ALTER TABLE ... RENAME COLUMN ... was first implemented in 3.25.0 but + // 3.26.0 was required so that foreign key constraints are updated as well +#if SQLITE_VERSION_NUMBER >= 3026000L + const bool bUseRenameColumn = (nActualFlags == ALTER_NAME_FLAG); + if( bUseRenameColumn ) + bUseRewriteSchemaMethod = false; +#else + constexpr bool bUseRenameColumn = false; +#endif if( m_poDS->SoftStartTransaction() != OGRERR_NONE ) return OGRERR_FAILURE; @@ -4826,7 +4871,8 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, /* column if renaming. We re-install some indexes afterwards. */ /* -------------------------------------------------------------------- */ std::unique_ptr oTriggers; - if( bRenameCol ) + // cppcheck-suppress knownConditionTrueFalse + if( bRenameCol && !bUseRenameColumn ) { char* pszSQL = sqlite3_mprintf( "SELECT name, type, sql FROM sqlite_master WHERE " @@ -4851,7 +4897,22 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, } } - if( !bUseFastMethod ) +#if SQLITE_VERSION_NUMBER >= 3026000L + if( bUseRenameColumn ) + { + if( eErr == OGRERR_NONE ) + { + CPLDebug("GPKG", "Running ALTER TABLE RENAME COLUMN"); + eErr = SQLCommand( m_poDS->GetDB(), + CPLString().Printf("ALTER TABLE \"%s\" RENAME COLUMN \"%s\" TO \"%s\"", + SQLEscapeName(m_pszTableName).c_str(), + SQLEscapeName(osOldColName).c_str(), + SQLEscapeName(osNewColName).c_str()).c_str() ); + } + } + else +#endif + if( !bUseRewriteSchemaMethod ) { /* -------------------------------------------------------------------- */ /* If we are within a transaction, we cannot use the method */ @@ -4969,13 +5030,26 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, } /* -------------------------------------------------------------------- */ -/* Run integrity check. */ +/* Run integrity check only if explicitly required. */ /* -------------------------------------------------------------------- */ - if( eErr == OGRERR_NONE ) + if( eErr == OGRERR_NONE && + CPLTestBool(CPLGetConfigOption("OGR_GPKG_INTEGRITY_CHECK", "NO")) ) { + CPLDebug("GPKG", "Running PRAGMA integrity_check"); eErr = m_poDS->PragmaCheck("integrity_check", "ok", 1); } +/* -------------------------------------------------------------------- */ +/* Otherwise check foreign key integrity if enforcement of foreign */ +/* kets constraint is enabled. */ +/* -------------------------------------------------------------------- */ + else if( eErr == OGRERR_NONE && + SQLGetInteger(m_poDS->GetDB(), "PRAGMA foreign_keys", nullptr) ) + { + CPLDebug("GPKG", "Running PRAGMA foreign_key_check"); + eErr = m_poDS->PragmaCheck("foreign_key_check", "", 0); + } + /* -------------------------------------------------------------------- */ /* Finish */ /* -------------------------------------------------------------------- */ @@ -4984,7 +5058,7 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, eErr = m_poDS->SoftCommitTransaction(); // We need to force database reopening due to schema change - if( eErr == OGRERR_NONE && bUseFastMethod && !m_poDS->ReOpenDB() ) + if( eErr == OGRERR_NONE && bUseRewriteSchemaMethod && !m_poDS->ReOpenDB() ) { CPLError(CE_Failure, CPLE_AppDefined, "Cannot reopen database"); eErr = OGRERR_FAILURE; @@ -5023,48 +5097,45 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, if( eErr == OGRERR_NONE ) { - OGRFieldDefn* poFieldDefn = - m_poFeatureDefn->GetFieldDefn(iFieldToAlter); - bool bRunDoSpecialProcessingForColumnCreation = false; bool bDeleteFromGpkgDataColumns = false; - if (nFlagsIn & ALTER_TYPE_FLAG) + if (nActualFlags & ALTER_TYPE_FLAG) { - if( poFieldDefn->GetSubType() == OFSTJSON && + if( poFieldDefnToAlter->GetSubType() == OFSTJSON && poNewFieldDefn->GetSubType() == OFSTNone ) { bDeleteFromGpkgDataColumns = true; } - else if ( poFieldDefn->GetSubType() == OFSTNone && + else if ( poFieldDefnToAlter->GetSubType() == OFSTNone && poNewFieldDefn->GetType() == OFTString && poNewFieldDefn->GetSubType() == OFSTJSON ) { bRunDoSpecialProcessingForColumnCreation = true; } - poFieldDefn->SetSubType(OFSTNone); - poFieldDefn->SetType(poNewFieldDefn->GetType()); - poFieldDefn->SetSubType(poNewFieldDefn->GetSubType()); + poFieldDefnToAlter->SetSubType(OFSTNone); + poFieldDefnToAlter->SetType(poNewFieldDefn->GetType()); + poFieldDefnToAlter->SetSubType(poNewFieldDefn->GetSubType()); } - if (nFlagsIn & ALTER_NAME_FLAG) + if (nActualFlags & ALTER_NAME_FLAG) { - poFieldDefn->SetName(poNewFieldDefn->GetNameRef()); + poFieldDefnToAlter->SetName(poNewFieldDefn->GetNameRef()); } - if (nFlagsIn & ALTER_WIDTH_PRECISION_FLAG) + if (nActualFlags & ALTER_WIDTH_PRECISION_FLAG) { - poFieldDefn->SetWidth(poNewFieldDefn->GetWidth()); - poFieldDefn->SetPrecision(poNewFieldDefn->GetPrecision()); + poFieldDefnToAlter->SetWidth(poNewFieldDefn->GetWidth()); + poFieldDefnToAlter->SetPrecision(poNewFieldDefn->GetPrecision()); } - if (nFlagsIn & ALTER_NULLABLE_FLAG) - poFieldDefn->SetNullable(poNewFieldDefn->IsNullable()); - if (nFlagsIn & ALTER_DEFAULT_FLAG) - poFieldDefn->SetDefault(poNewFieldDefn->GetDefault()); - if (nFlagsIn & ALTER_UNIQUE_FLAG) - poFieldDefn->SetUnique(poNewFieldDefn->IsUnique()); - if ( (nFlagsIn & ALTER_DOMAIN_FLAG) && - poFieldDefn->GetDomainName() != poNewFieldDefn->GetDomainName() ) + if (nActualFlags & ALTER_NULLABLE_FLAG) + poFieldDefnToAlter->SetNullable(poNewFieldDefn->IsNullable()); + if (nActualFlags & ALTER_DEFAULT_FLAG) + poFieldDefnToAlter->SetDefault(poNewFieldDefn->GetDefault()); + if (nActualFlags & ALTER_UNIQUE_FLAG) + poFieldDefnToAlter->SetUnique(poNewFieldDefn->IsUnique()); + if ( (nActualFlags & ALTER_DOMAIN_FLAG) && + poFieldDefnToAlter->GetDomainName() != poNewFieldDefn->GetDomainName() ) { - if( !poFieldDefn->GetDomainName().empty() ) + if( !poFieldDefnToAlter->GetDomainName().empty() ) { bDeleteFromGpkgDataColumns = true; } @@ -5074,7 +5145,7 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, bRunDoSpecialProcessingForColumnCreation = true; } - poFieldDefn->SetDomainName(poNewFieldDefn->GetDomainName()); + poFieldDefnToAlter->SetDomainName(poNewFieldDefn->GetDomainName()); } if( bDeleteFromGpkgDataColumns ) @@ -5084,14 +5155,14 @@ OGRErr OGRGeoPackageTableLayer::AlterFieldDefn( int iFieldToAlter, "lower(table_name) = lower('%q') AND " "lower(column_name) = lower('%q')", m_pszTableName, - poFieldDefn->GetNameRef() ); + poFieldDefnToAlter->GetNameRef() ); eErr = SQLCommand( m_poDS->GetDB(), pszSQL ); sqlite3_free(pszSQL); } if( bRunDoSpecialProcessingForColumnCreation ) { - if( !DoSpecialProcessingForColumnCreation(poFieldDefn) ) + if( !DoSpecialProcessingForColumnCreation(poFieldDefnToAlter) ) eErr = OGRERR_FAILURE; } diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h b/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h index d22b17743606..755e63cb7b06 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h +++ b/ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h @@ -175,6 +175,8 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL: public GDALPamDataset OGRErr SoftStartTransaction(); OGRErr SoftCommitTransaction(); OGRErr SoftRollbackTransaction(); + + OGRErr PragmaCheck(const char * pszPragma, const char * pszExpected, int nRowsExpected); }; /************************************************************************/ diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp index 7915ec5c5d48..f87a2e1f45e4 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp +++ b/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp @@ -425,6 +425,56 @@ void OGRSQLiteBaseDataSource::CloseDB() } } +/* Returns the first row of first column of SQL as integer */ +OGRErr OGRSQLiteBaseDataSource::PragmaCheck( + const char * pszPragma, const char * pszExpected, int nRowsExpected ) +{ + CPLAssert( pszPragma != nullptr ); + CPLAssert( pszExpected != nullptr ); + CPLAssert( nRowsExpected >= 0 ); + + char **papszResult = nullptr; + int nRowCount = 0; + int nColCount = 0; + char *pszErrMsg = nullptr; + + int rc = sqlite3_get_table( + hDB, + CPLSPrintf("PRAGMA %s", pszPragma), + &papszResult, &nRowCount, &nColCount, &pszErrMsg ); + + if( rc != SQLITE_OK ) + { + CPLError( CE_Failure, CPLE_AppDefined, + "Unable to execute PRAGMA %s: %s", pszPragma, + pszErrMsg ? pszErrMsg : "(null)" ); + sqlite3_free( pszErrMsg ); + return OGRERR_FAILURE; + } + + if ( nRowCount != nRowsExpected ) + { + CPLError( CE_Failure, CPLE_AppDefined, + "bad result for PRAGMA %s, got %d rows, expected %d", + pszPragma, nRowCount, nRowsExpected ); + sqlite3_free_table(papszResult); + return OGRERR_FAILURE; + } + + if ( nRowCount > 0 && ! EQUAL(papszResult[1], pszExpected) ) + { + CPLError( CE_Failure, CPLE_AppDefined, + "invalid %s (expected '%s', got '%s')", + pszPragma, pszExpected, papszResult[1]); + sqlite3_free_table(papszResult); + return OGRERR_FAILURE; + } + + sqlite3_free_table(papszResult); + + return OGRERR_NONE; +} + /************************************************************************/ /* OGRSQLiteDataSource() */ /************************************************************************/ diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqliteselectlayer.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqliteselectlayer.cpp index 63739dd659d8..699822e09bdb 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqliteselectlayer.cpp +++ b/ogr/ogrsf_frmts/sqlite/ogrsqliteselectlayer.cpp @@ -288,8 +288,12 @@ GIntBig OGRSQLiteSelectLayerCommonBehaviour::GetFeatureCount( int bForce ) m_osSQLCurrent.ifind(" EXCEPT ") == std::string::npos ) return 1; - if( m_poLayer->GetFeatureQuery() != nullptr || (m_poLayer->GetFilterGeom() != nullptr && !m_bSpatialFilterInSQL) ) + if( m_poLayer->GetFeatureQuery() != nullptr || + (m_poLayer->GetFilterGeom() != nullptr && !m_bSpatialFilterInSQL) || + STARTS_WITH_CI(m_osSQLCurrent.c_str(), "PRAGMA table_info(") ) + { return m_poLayer->BaseGetFeatureCount(bForce); + } CPLString osFeatureCountSQL("SELECT COUNT(*) FROM ("); osFeatureCountSQL += m_osSQLCurrent; diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp index 8f72765ee755..7f46477a265f 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp +++ b/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp @@ -1978,6 +1978,19 @@ OGRErr OGRSQLiteTableLayer::DeleteField( int iFieldToDelete ) ResetReading(); + if( m_poDS->SoftStartTransaction() != OGRERR_NONE ) + return OGRERR_FAILURE; + + // ALTER TABLE ... DROP COLUMN ... was first implemented in 3.35.0 but + // there was bug fixes related to it until 3.35.5 +#if SQLITE_VERSION_NUMBER >= 3035005L + const char* pszFieldName = + m_poFeatureDefn->GetFieldDefn(iFieldToDelete)->GetNameRef(); + OGRErr eErr = SQLCommand( m_poDS->GetDB(), + CPLString().Printf("ALTER TABLE \"%s\" DROP COLUMN \"%s\"", + SQLEscapeName(m_pszTableName).c_str(), + SQLEscapeName(pszFieldName).c_str()).c_str() ); +#else /* -------------------------------------------------------------------- */ /* Build list of old fields, and the list of new fields. */ /* -------------------------------------------------------------------- */ @@ -2015,16 +2028,39 @@ OGRErr OGRSQLiteTableLayer::DeleteField( int iFieldToDelete ) CPLFree( pszFieldListForSelect ); CPLFree( pszNewFieldList ); +#endif - if (eErr != OGRERR_NONE) - return eErr; +/* -------------------------------------------------------------------- */ +/* Check foreign key integrity if enforcement of foreign keys */ +/* constraint is enabled. */ +/* -------------------------------------------------------------------- */ + if( eErr == OGRERR_NONE && + SQLGetInteger(m_poDS->GetDB(), "PRAGMA foreign_keys", nullptr) ) + { + CPLDebug("SQLite", "Running PRAGMA foreign_key_check"); + eErr = m_poDS->PragmaCheck("foreign_key_check", "", 0); + } /* -------------------------------------------------------------------- */ /* Finish */ /* -------------------------------------------------------------------- */ - eErr = m_poFeatureDefn->DeleteFieldDefn( iFieldToDelete ); - RecomputeOrdinals(); + if( eErr == OGRERR_NONE) + { + eErr = m_poDS->SoftCommitTransaction(); + if( eErr == OGRERR_NONE) + { + eErr = m_poFeatureDefn->DeleteFieldDefn( iFieldToDelete ); + + RecomputeOrdinals(); + + ResetReading(); + } + } + else + { + m_poDS->SoftRollbackTransaction(); + } return eErr; } @@ -2057,104 +2093,171 @@ OGRErr OGRSQLiteTableLayer::AlterFieldDefn( int iFieldToAlter, OGRFieldDefn* poN ResetReading(); /* -------------------------------------------------------------------- */ -/* Build list of old fields, and the list of new fields. */ +/* Check that the new column name is not a duplicate. */ /* -------------------------------------------------------------------- */ - char *pszNewFieldList = nullptr; - char *pszFieldListForSelect = nullptr; - size_t nBufLen = 0; - InitFieldListForRecrerate(pszNewFieldList, pszFieldListForSelect, - nBufLen, - static_cast(strlen(poNewFieldDefn->GetNameRef())) + - 50 + - (poNewFieldDefn->GetDefault() ? static_cast(strlen(poNewFieldDefn->GetDefault())) : 0) - ); + OGRFieldDefn* poFieldDefnToAlter = m_poFeatureDefn->GetFieldDefn(iFieldToAlter); + const CPLString osOldColName( poFieldDefnToAlter->GetNameRef() ); + const CPLString osNewColName( (nFlagsIn & ALTER_NAME_FLAG) ? + CPLString(poNewFieldDefn->GetNameRef()) : + osOldColName ); - for( int iField = 0; iField < m_poFeatureDefn->GetFieldCount(); iField++ ) + const bool bRenameCol = osOldColName != osNewColName; + if( bRenameCol ) { - OGRFieldDefn *poFldDefn = m_poFeatureDefn->GetFieldDefn(iField); + if( (m_pszFIDColumn && + strcmp(poNewFieldDefn->GetNameRef(), m_pszFIDColumn) == 0) || + (GetGeomType() != wkbNone && + strcmp(poNewFieldDefn->GetNameRef(), + m_poFeatureDefn->GetGeomFieldDefn(0)->GetNameRef()) == 0) || + m_poFeatureDefn->GetFieldIndex(poNewFieldDefn->GetNameRef()) >= 0 ) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Field name %s is already used for another field", + poNewFieldDefn->GetNameRef()); + return OGRERR_FAILURE; + } + } - snprintf( pszFieldListForSelect+strlen(pszFieldListForSelect), - nBufLen-strlen(pszFieldListForSelect), - ", \"%s\"", SQLEscapeName(poFldDefn->GetNameRef()).c_str() ); +/* -------------------------------------------------------------------- */ +/* Build the modified field definition from the flags. */ +/* -------------------------------------------------------------------- */ + OGRFieldDefn oTmpFieldDefn(poFieldDefnToAlter); + int nActualFlags = 0; + if( bRenameCol ) + { + nActualFlags |= ALTER_NAME_FLAG; + oTmpFieldDefn.SetName(poNewFieldDefn->GetNameRef()); + } + if( (nFlagsIn & ALTER_TYPE_FLAG) != 0 && + (poFieldDefnToAlter->GetType() != poNewFieldDefn->GetType() || + poFieldDefnToAlter->GetSubType() != poNewFieldDefn->GetSubType()) ) + { + nActualFlags |= ALTER_TYPE_FLAG; + oTmpFieldDefn.SetSubType(OFSTNone); + oTmpFieldDefn.SetType(poNewFieldDefn->GetType()); + oTmpFieldDefn.SetSubType(poNewFieldDefn->GetSubType()); + } + if ( (nFlagsIn & ALTER_WIDTH_PRECISION_FLAG) != 0 && + (poFieldDefnToAlter->GetWidth() != poNewFieldDefn->GetWidth() || + poFieldDefnToAlter->GetPrecision() != poNewFieldDefn->GetPrecision()) ) + { + nActualFlags |= ALTER_WIDTH_PRECISION_FLAG; + oTmpFieldDefn.SetWidth(poNewFieldDefn->GetWidth()); + oTmpFieldDefn.SetPrecision(poNewFieldDefn->GetPrecision()); + } + if( (nFlagsIn & ALTER_NULLABLE_FLAG) != 0 && + poFieldDefnToAlter->IsNullable() != poNewFieldDefn->IsNullable() ) + { + nActualFlags |= ALTER_NULLABLE_FLAG; + oTmpFieldDefn.SetNullable(poNewFieldDefn->IsNullable()); + } + if( (nFlagsIn & ALTER_DEFAULT_FLAG) != 0 && + !( (poFieldDefnToAlter->GetDefault() == nullptr && poNewFieldDefn->GetDefault() == nullptr) || + (poFieldDefnToAlter->GetDefault() != nullptr && poNewFieldDefn->GetDefault() != nullptr && + strcmp(poFieldDefnToAlter->GetDefault(), poNewFieldDefn->GetDefault()) == 0) ) ) + { + nActualFlags |= ALTER_DEFAULT_FLAG; + oTmpFieldDefn.SetDefault(poNewFieldDefn->GetDefault()); + } + if( (nFlagsIn & ALTER_UNIQUE_FLAG) != 0 && + poFieldDefnToAlter->IsUnique() != poNewFieldDefn->IsUnique() ) + { + nActualFlags |= ALTER_UNIQUE_FLAG; + oTmpFieldDefn.SetUnique( poNewFieldDefn->IsUnique()); + } - if (iField == iFieldToAlter) + // ALTER TABLE ... RENAME COLUMN ... was first implemented in 3.25.0 but + // 3.26.0 was required so that foreign key constraints are updated as well +#if SQLITE_VERSION_NUMBER >= 3026000L + if( nActualFlags == ALTER_NAME_FLAG ) + { + CPLDebug("SQLite", "Running ALTER TABLE RENAME COLUMN"); + OGRErr eErr = SQLCommand( m_poDS->GetDB(), + CPLString().Printf("ALTER TABLE \"%s\" RENAME COLUMN \"%s\" TO \"%s\"", + SQLEscapeName(m_pszTableName).c_str(), + SQLEscapeName(osOldColName).c_str(), + SQLEscapeName(osNewColName).c_str()).c_str() ); + + if (eErr != OGRERR_NONE) + return eErr; + } + else +#endif + { +/* -------------------------------------------------------------------- */ +/* Build list of old fields, and the list of new fields. */ +/* -------------------------------------------------------------------- */ + char *pszNewFieldList = nullptr; + char *pszFieldListForSelect = nullptr; + size_t nBufLen = 0; + + InitFieldListForRecrerate(pszNewFieldList, pszFieldListForSelect, + nBufLen, + static_cast(strlen(poNewFieldDefn->GetNameRef())) + + 50 + + (poNewFieldDefn->GetDefault() ? static_cast(strlen(poNewFieldDefn->GetDefault())) : 0) + ); + + for( int iField = 0; iField < m_poFeatureDefn->GetFieldCount(); iField++ ) { - OGRFieldDefn oTmpFieldDefn(poFldDefn); - if( (nFlagsIn & ALTER_NAME_FLAG) ) - oTmpFieldDefn.SetName(poNewFieldDefn->GetNameRef()); - if( (nFlagsIn & ALTER_TYPE_FLAG) ) - { - oTmpFieldDefn.SetSubType(OFSTNone); - oTmpFieldDefn.SetType(poNewFieldDefn->GetType()); - oTmpFieldDefn.SetSubType(poNewFieldDefn->GetSubType()); - } - if (nFlagsIn & ALTER_WIDTH_PRECISION_FLAG) - { - oTmpFieldDefn.SetWidth(poNewFieldDefn->GetWidth()); - oTmpFieldDefn.SetPrecision(poNewFieldDefn->GetPrecision()); - } - if( (nFlagsIn & ALTER_NULLABLE_FLAG) ) - { - oTmpFieldDefn.SetNullable(poNewFieldDefn->IsNullable()); - } - if( (nFlagsIn & ALTER_UNIQUE_FLAG) ) - { - oTmpFieldDefn.SetUnique(poNewFieldDefn->IsUnique()); - } - if( (nFlagsIn & ALTER_DEFAULT_FLAG) ) - { - oTmpFieldDefn.SetDefault(poNewFieldDefn->GetDefault()); - } + OGRFieldDefn *poFldDefn = m_poFeatureDefn->GetFieldDefn(iField); + + snprintf( pszFieldListForSelect+strlen(pszFieldListForSelect), + nBufLen-strlen(pszFieldListForSelect), + ", \"%s\"", SQLEscapeName(poFldDefn->GetNameRef()).c_str() ); - snprintf( pszNewFieldList+strlen(pszNewFieldList), - nBufLen-strlen(pszNewFieldList), - ", '%s' %s", - SQLEscapeLiteral(oTmpFieldDefn.GetNameRef()).c_str(), - FieldDefnToSQliteFieldDefn(&oTmpFieldDefn).c_str() ); - if ( (nFlagsIn & ALTER_NAME_FLAG) && - oTmpFieldDefn.GetType() == OFTString && - CSLFindString(m_papszCompressedColumns, poFldDefn->GetNameRef()) >= 0 ) + if (iField == iFieldToAlter) { snprintf( pszNewFieldList+strlen(pszNewFieldList), - nBufLen-strlen(pszNewFieldList), "_deflate"); + nBufLen-strlen(pszNewFieldList), + ", '%s' %s", + SQLEscapeLiteral(oTmpFieldDefn.GetNameRef()).c_str(), + FieldDefnToSQliteFieldDefn(&oTmpFieldDefn).c_str() ); + if ( (nFlagsIn & ALTER_NAME_FLAG) && + oTmpFieldDefn.GetType() == OFTString && + CSLFindString(m_papszCompressedColumns, poFldDefn->GetNameRef()) >= 0 ) + { + snprintf( pszNewFieldList+strlen(pszNewFieldList), + nBufLen-strlen(pszNewFieldList), "_deflate"); + } + if( !oTmpFieldDefn.IsNullable() ) + snprintf( pszNewFieldList+strlen(pszNewFieldList), + nBufLen-strlen(pszNewFieldList)," NOT NULL" ); + if( oTmpFieldDefn.IsUnique() ) + snprintf( pszNewFieldList+strlen(pszNewFieldList), + nBufLen-strlen(pszNewFieldList)," UNIQUE" ); + if( oTmpFieldDefn.GetDefault() ) + { + snprintf( pszNewFieldList+strlen(pszNewFieldList), + nBufLen-strlen(pszNewFieldList)," DEFAULT %s", + oTmpFieldDefn.GetDefault()); + } } - if( !oTmpFieldDefn.IsNullable() ) - snprintf( pszNewFieldList+strlen(pszNewFieldList), - nBufLen-strlen(pszNewFieldList)," NOT NULL" ); - if( oTmpFieldDefn.IsUnique() ) - snprintf( pszNewFieldList+strlen(pszNewFieldList), - nBufLen-strlen(pszNewFieldList)," UNIQUE" ); - if( oTmpFieldDefn.GetDefault() ) + else { - snprintf( pszNewFieldList+strlen(pszNewFieldList), - nBufLen-strlen(pszNewFieldList)," DEFAULT %s", - oTmpFieldDefn.GetDefault()); + AddColumnDef(pszNewFieldList, nBufLen, poFldDefn); } } - else - { - AddColumnDef(pszNewFieldList, nBufLen, poFldDefn); - } - } /* -------------------------------------------------------------------- */ /* Recreate table. */ /* -------------------------------------------------------------------- */ - CPLString osErrorMsg; - osErrorMsg.Printf("Failed to alter field %s from table %s", - m_poFeatureDefn->GetFieldDefn(iFieldToAlter)->GetNameRef(), - m_poFeatureDefn->GetName()); + CPLString osErrorMsg; + osErrorMsg.Printf("Failed to alter field %s from table %s", + m_poFeatureDefn->GetFieldDefn(iFieldToAlter)->GetNameRef(), + m_poFeatureDefn->GetName()); - OGRErr eErr = RecreateTable(pszFieldListForSelect, - pszNewFieldList, - osErrorMsg.c_str()); + OGRErr eErr = RecreateTable(pszFieldListForSelect, + pszNewFieldList, + osErrorMsg.c_str()); - CPLFree( pszFieldListForSelect ); - CPLFree( pszNewFieldList ); + CPLFree( pszFieldListForSelect ); + CPLFree( pszNewFieldList ); - if (eErr != OGRERR_NONE) - return eErr; + if (eErr != OGRERR_NONE) + return eErr; + } /* -------------------------------------------------------------------- */ /* Finish */ @@ -2162,7 +2265,7 @@ OGRErr OGRSQLiteTableLayer::AlterFieldDefn( int iFieldToAlter, OGRFieldDefn* poN OGRFieldDefn* poFieldDefn = m_poFeatureDefn->GetFieldDefn(iFieldToAlter); - if (nFlagsIn & ALTER_TYPE_FLAG) + if (nActualFlags & ALTER_TYPE_FLAG) { int iIdx = 0; if( poNewFieldDefn->GetType() != OFTString && @@ -2176,7 +2279,7 @@ OGRErr OGRSQLiteTableLayer::AlterFieldDefn( int iFieldToAlter, OGRFieldDefn* poN poFieldDefn->SetType(poNewFieldDefn->GetType()); poFieldDefn->SetSubType(poNewFieldDefn->GetSubType()); } - if (nFlagsIn & ALTER_NAME_FLAG) + if (nActualFlags & ALTER_NAME_FLAG) { const int iIdx = CSLFindString(m_papszCompressedColumns, poFieldDefn->GetNameRef()); @@ -2188,14 +2291,14 @@ OGRErr OGRSQLiteTableLayer::AlterFieldDefn( int iFieldToAlter, OGRFieldDefn* poN } poFieldDefn->SetName(poNewFieldDefn->GetNameRef()); } - if (nFlagsIn & ALTER_WIDTH_PRECISION_FLAG) + if (nActualFlags & ALTER_WIDTH_PRECISION_FLAG) { poFieldDefn->SetWidth(poNewFieldDefn->GetWidth()); poFieldDefn->SetPrecision(poNewFieldDefn->GetPrecision()); } - if (nFlagsIn & ALTER_NULLABLE_FLAG) + if (nActualFlags & ALTER_NULLABLE_FLAG) poFieldDefn->SetNullable(poNewFieldDefn->IsNullable()); - if (nFlagsIn & ALTER_DEFAULT_FLAG) + if (nActualFlags & ALTER_DEFAULT_FLAG) poFieldDefn->SetDefault(poNewFieldDefn->GetDefault()); return OGRERR_NONE;