Changed insert/update of GENERATED fields

Changed the way GENERATED fields are inserted/updated. Previously we used the DEFAULT keyword for INSERTing; now, we expose the generating expression to the user, but ommit the field when inserting or updating a feature.
qgis · Jun 2, 2020 · e16ece7 · e16ece7
1 parent a6208da
commit e16ece7
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 5 deletions.
diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -920,7 +920,7 @@ bool QgsPostgresProvider::loadFields()
         notNullMap[attrelid][attnum] = attNotNull;
         uniqueMap[attrelid][attnum] = uniqueConstraint;
         identityMap[attrelid][attnum] = attIdentity.isEmpty() ? " " : attIdentity;
-        generatedMap[attrelid][attnum] = attGenerated;
+        generatedMap[attrelid][attnum] = attGenerated == "s" ? defVal : "";
       }
     }
   }
@@ -2081,10 +2081,14 @@ QString QgsPostgresProvider::defaultValueClause( int fieldId ) const
 
   // with generated columns (PostgreSQL 12+), the provider will ALWAYS evaluate the default values.
   // The only acceptable value for such columns on INSERT or UPDATE clauses is the keyword "DEFAULT".
+  // Here, we return the expression used to generate the field value, so the
+  // user can see what is happening when inserting a new feature.
+  // On inserting a new feature or updating a generated field, this is
+  // ommited from the generated queries.
   // See https://www.postgresql.org/docs/12/ddl-generated-columns.html
   if ( !genVal.isEmpty() )
   {
-    return "DEFAULT";
+    return defVal;
   }
 
   if ( !providerProperty( EvaluateDefaultValues, false ).toBool() && !defVal.isEmpty() )
@@ -2338,6 +2342,13 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
         continue;
 
       QString fieldname = mAttributeFields.at( idx ).name();
+
+      if ( mGeneratedValues.contains( idx ) && !mGeneratedValues.value( idx, QString() ).isEmpty() )
+      {
+        QgsDebugMsg( QStringLiteral( "Skipping field %1 (idx %2) which is GENERATED." ).arg( fieldname, idx ) );
+        continue;
+      }
+
       QString fieldTypeName = mAttributeFields.at( idx ).typeName();
 
       QgsDebugMsgLevel( "Checking field against: " + fieldname, 2 );
@@ -2941,6 +2952,7 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
 
       // cycle through the changed attributes of the feature
       QString delim;
+      int numChangedFields = 0;
       for ( QgsAttributeMap::const_iterator siter = attrs.constBegin(); siter != attrs.constEnd(); ++siter )
       {
         try
@@ -2949,6 +2961,14 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
 
           pkChanged = pkChanged || mPrimaryKeyAttrs.contains( siter.key() );
 
+          if ( mGeneratedValues.contains( siter.key() ) && !mGeneratedValues.value( siter.key(), QString() ).isEmpty() )
+          {
+            pushError( tr( "Changing the value of GENERATED field %1 is not allowed." ).arg( fld.name() ) );
+            continue;
+          }
+
+          numChangedFields++;
+
           sql += delim + QStringLiteral( "%1=" ).arg( quotedIdentifier( fld.name() ) );
           delim = ',';
 
@@ -2990,9 +3010,14 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
 
       sql += QStringLiteral( " WHERE %1" ).arg( whereClause( fid ) );
 
-      QgsPostgresResult result( conn->PQexec( sql ) );
-      if ( result.PQresultStatus() != PGRES_COMMAND_OK && result.PQresultStatus() != PGRES_TUPLES_OK )
-        throw PGException( result );
+      // Don't try to UPDATE an empty set of values (might happen if the table only has GENERATED fields,
+      // or if the user only changed GENERATED fields in the form/attribute table.
+      if ( numChangedFields > 0 )
+      {
+        QgsPostgresResult result( conn->PQexec( sql ) );
+        if ( result.PQresultStatus() != PGRES_COMMAND_OK && result.PQresultStatus() != PGRES_TUPLES_OK )
+          throw PGException( result );
+      }
 
       // update feature id map if key was changed
       if ( pkChanged && mPrimaryKeyType == PktFidMap )

diff --git a/tests/testdata/provider/testdata_pg_12_generated.sql b/tests/testdata/provider/testdata_pg_12_generated.sql
@@ -3,6 +3,7 @@
 
 CREATE TABLE qgis_test.test_gen_col (
   id SERIAL PRIMARY KEY,
+  name varchar(32),
   geom GEOMETRY('Polygon', 4326) NOT NULL,
   cent GEOMETRY('Point', 4326) NOT NULL GENERATED ALWAYS AS ( st_centroid(geom) ) STORED,
   poly_area FLOAT NOT NULL GENERATED ALWAYS AS ( st_area(st_transform(geom, 31979)) ) STORED