Add support for Oracle virtual (generated) column

qgis · Sep 1, 2020 · 2970697 · 2970697
1 parent 97b56fe
commit 2970697
Show file tree

Hide file tree

Showing 9 changed files with 146 additions and 73 deletions.
diff --git a/src/providers/oracle/qgsoracleconn.cpp b/src/providers/oracle/qgsoracleconn.cpp
@@ -972,6 +972,24 @@ bool QgsOracleConn::hasSpatial()
   return mHasSpatial;
 }
 
+int QgsOracleConn::version()
+{
+  QSqlQuery qry( mDatabase );
+  QString sql = QStringLiteral( "SELECT VERSION FROM PRODUCT_COMPONENT_VERSION" );
+  if ( exec( qry, sql, QVariantList() ) && qry.next() )
+  {
+    return qry.value( 0 ).toString().split( '.' ).at( 0 ).toInt();
+  }
+  else
+  {
+    QgsMessageLog::logMessage( tr( "Unable to execute the query.\nThe error message from the database was:\n%1.\nSQL: %2" )
+                               .arg( qry.lastError().text() )
+                               .arg( qry.lastQuery() ), tr( "Oracle" ) );
+    return -1;
+  }
+}
+
+
 QString QgsOracleConn::currentUser()
 {
   QMutexLocker locker( &mLock );

diff --git a/src/providers/oracle/qgsoracleconn.h b/src/providers/oracle/qgsoracleconn.h
@@ -179,6 +179,11 @@ class QgsOracleConn : public QObject
 
     bool hasSpatial();
 
+    /**
+     * \returns Oracle database version, -1 in an error occured
+     */
+    int version();
+
     static const int sGeomTypeSelectLimit;
 
     static QString displayStringForWkbType( QgsWkbTypes::Type wkbType );

diff --git a/src/providers/oracle/qgsoracleprovider.cpp b/src/providers/oracle/qgsoracleprovider.cpp
@@ -61,6 +61,7 @@ QgsOracleProvider::QgsOracleProvider( QString const &uri, const ProviderOptions
   , mRequestedGeomType( QgsWkbTypes::Unknown )
   , mHasSpatialIndex( false )
   , mSpatialIndexName( QString() )
+  , mOracleVersion( -1 )
   , mShared( new QgsOracleSharedData )
 {
   static int geomMetaType = -1;
@@ -144,8 +145,8 @@ QgsOracleProvider::QgsOracleProvider( QString const &uri, const ProviderOptions
 
   mLayerExtent.setMinimal();
 
-  // get always generated key
-  if ( !determineAlwaysGeneratedKeys() )
+  mOracleVersion = connectionRO()->version();
+  if ( mOracleVersion < 0 )
   {
     mValid = false;
     disconnectDb();
@@ -555,6 +556,7 @@ bool QgsOracleProvider::loadFields()
   QMap<QString, QString> comments;
   QMap<QString, QString> types;
   QMap<QString, QVariant> defvalues;
+  QMap<QString, bool> alwaysGenerated;
 
   if ( !mIsQuery )
   {
@@ -608,9 +610,16 @@ bool QgsOracleProvider::loadFields()
                  ",t.data_scale"
                  ",t.char_length"
                  ",t.char_used"
-                 ",t.data_default"
-                 " FROM all_tab_columns t"
-                 " WHERE t.owner=? AND t.table_name=?" ) ;
+                 ",t.data_default" +
+                 QString( mOracleVersion >= 12 ?
+                          ",CASE WHEN t.virtual_column = 'YES' OR a.generation_type = 'ALWAYS' THEN 'YES' ELSE 'NO' END" :
+                          ",t.virtual_column" ) +
+                 " FROM all_tab_cols t" +
+                 QString( mOracleVersion >= 12 ?
+                          " LEFT JOIN all_tab_identity_cols a ON a.column_name = t.column_name AND a.owner = t.owner AND a.table_name = t.table_name" :
+                          "" ) +
+                 " WHERE t.owner=? AND t.table_name=?"
+                 " AND t.hidden_column='NO'" ) ;
     args << mOwnerName << mTableName;
 
     if ( !mGeometryColumn.isEmpty() )
@@ -637,6 +646,7 @@ bool QgsOracleProvider::loadFields()
         int clength       = qry.value( 4 ).toInt();
         bool cused        = qry.value( 5 ).toString() == "C";
         QVariant defValue = qry.value( 6 );
+        bool alwaysGen    = qry.value( 7 ).toString() == "YES";
 
         if ( type == "CHAR" || type == "VARCHAR2" || type == "VARCHAR" )
         {
@@ -663,6 +673,7 @@ bool QgsOracleProvider::loadFields()
         }
 
         defvalues.insert( name, defValue );
+        alwaysGenerated.insert( name, alwaysGen );
       }
     }
     else
@@ -767,6 +778,7 @@ bool QgsOracleProvider::loadFields()
 
     mAttributeFields.append( newField );
     mDefaultValues.append( defvalues.value( field.name(), QVariant() ) );
+    mAlwaysGenerated.append( alwaysGenerated.value( field.name(), false ) );
   }
 
   return true;
@@ -1007,61 +1019,6 @@ void QgsOracleProvider::determinePrimaryKeyFromUriKeyColumn()
   }
 }
 
-bool QgsOracleProvider::determineAlwaysGeneratedKeys()
-{
-  if ( !loadFields() )
-  {
-    return false;
-  }
-
-  mValid = true;
-  // check to see if there is an unique index on the relation, which
-  // can be used as a key into the table. Primary keys are always
-  // unique indices, so we catch them as well.
-  QgsOracleConn *conn = connectionRO();
-  QSqlQuery qry( *conn );
-  QString sql = QStringLiteral( "SELECT VERSION FROM PRODUCT_COMPONENT_VERSION" );
-  if ( exec( qry, sql, QVariantList() ) && qry.next() )
-  {
-    // Identity type is a feature since Oracle 12 version, otherwise all_tab_identity_cols table doesn't exist
-    if ( qry.value( 0 ).toString().split( '.' ).at( 0 ).toInt() >= 12 )
-    {
-      QgsDebugMsgLevel( QStringLiteral( "Oracle version : %1" ).arg( qry.value( 0 ).toString().split( '.' ).at( 0 ) ), 2 );
-      QString sql = QStringLiteral( "SELECT a.column_name "
-                                    "FROM all_tab_identity_cols a "
-                                    "WHERE a.owner = '%1' "
-                                    "AND a.table_name = '%2' "
-                                    "AND a.generation_type = 'ALWAYS'" ).arg( mOwnerName ).arg( mTableName );
-
-      if ( exec( qry, sql, QVariantList() ) )
-      {
-        while ( qry.next() )
-        {
-          if ( mAttributeFields.names().contains( qry.value( 0 ).toString() ) )
-          {
-            mAlwaysGeneratedKeyAttrs.append( mAttributeFields.indexOf( qry.value( 0 ).toString() ) );
-          }
-        }
-      }
-      else
-      {
-        QgsMessageLog::logMessage( tr( "Unable to execute the query.\nThe error message from the database was:\n%1.\nSQL: %2" )
-                                   .arg( qry.lastError().text() )
-                                   .arg( qry.lastQuery() ), tr( "Oracle" ) );
-        mValid = false;
-      }
-    }
-  }
-  else
-  {
-    QgsMessageLog::logMessage( tr( "Unable to execute the query.\nThe error message from the database was:\n%1.\nSQL: %2" )
-                               .arg( qry.lastError().text() )
-                               .arg( qry.lastQuery() ), tr( "Oracle" ) );
-    mValid = false;
-  }
-  return mValid;
-}
-
 bool QgsOracleProvider::uniqueData( QString query, QString colName )
 {
   Q_UNUSED( query )
@@ -1320,7 +1277,7 @@ bool QgsOracleProvider::addFeatures( QgsFeatureList &flist, QgsFeatureSink::Flag
 
   try
   {
-    QSqlQuery ins( db ), getfid( db ), identitytype( db );
+    QSqlQuery ins( db ), getfid( db );
 
     if ( !conn->begin( db ) )
     {
@@ -1351,7 +1308,7 @@ bool QgsOracleProvider::addFeatures( QgsFeatureList &flist, QgsFeatureSink::Flag
       {
         QgsField fld = field( idx );
         keys += kdelim + quotedIdentifier( fld.name() );
-        if ( mAlwaysGeneratedKeyAttrs.contains( idx ) )
+        if ( mAlwaysGenerated.at( idx ) )
           continue;
         insert += delim + quotedIdentifier( fld.name() );
         values += delim + '?';
@@ -1374,7 +1331,7 @@ bool QgsOracleProvider::addFeatures( QgsFeatureList &flist, QgsFeatureSink::Flag
     for ( int idx = 0; idx < std::min( attributevec.size(), mAttributeFields.size() ); ++idx )
     {
       QVariant v = attributevec[idx];
-      if ( mAlwaysGeneratedKeyAttrs.contains( idx ) )
+      if ( mAlwaysGenerated.at( idx ) )
         continue;
       if ( !v.isValid() )
         continue;
@@ -1862,6 +1819,12 @@ bool QgsOracleProvider::changeAttributeValues( const QgsChangedAttributesMap &at
         {
           QgsField fld = field( siter.key() );
 
+          if ( mAlwaysGenerated.at( siter.key() ) )
+          {
+            QgsLogger::warning( tr( "Changing the value of GENERATED field %1 is not allowed." ).arg( fld.name() ) );
+            continue;
+          }
+
           pkChanged = pkChanged || mPrimaryKeyAttrs.contains( siter.key() );
 
           sql += delim + QString( "%1=?" ).arg( quotedIdentifier( fld.name() ) );
@@ -1875,6 +1838,10 @@ bool QgsOracleProvider::changeAttributeValues( const QgsChangedAttributesMap &at
         }
       }
 
+      if ( params.isEmpty() )
+        // Don't try to UPDATE an empty set of values (might happen if only GENERATED fields has changed
+        continue;
+
       QVariantList args;
       sql += QString( " WHERE %1" ).arg( whereClause( fid, args ) );
 
@@ -3104,8 +3071,6 @@ QgsVectorLayerExporter::ExportError QgsOracleProvider::createEmptyLayer(
 
         srid = qry.value( 0 ).toInt();
 
-        QString sql;
-
         if ( !exec( qry, QStringLiteral( "INSERT"
                                          " INTO sdo_coord_ref_system(srid,coord_ref_sys_name,coord_ref_sys_kind,legacy_wktext,is_valid,is_legacy,information_source)"
                                          " VALUES (?,?,?,?,'TRUE','TRUE','GDAL/OGR via QGIS')" ),

diff --git a/src/providers/oracle/qgsoracleprovider.h b/src/providers/oracle/qgsoracleprovider.h
@@ -135,11 +135,6 @@ class QgsOracleProvider final: public QgsVectorDataProvider
      */
     void determinePrimaryKeyFromUriKeyColumn();
 
-    /**
-     * Determine the always generated identity fields
-     */
-    bool determineAlwaysGeneratedKeys();
-
     QgsFields fields() const override;
     QString dataComment() const override;
 
@@ -265,9 +260,10 @@ class QgsOracleProvider final: public QgsVectorDataProvider
     QString mPrimaryKeyDefault;
 
     /**
-     * List of always generated key attributes
+     * For each attributes, true if the attribute is always generated (virtual column
+     * and identity always generated columns)
      */
-    QList<int> mAlwaysGeneratedKeyAttrs;
+    QList<bool> mAlwaysGenerated;
 
     QString mGeometryColumn;           //!< Name of the geometry column
     mutable QgsRectangle mLayerExtent; //!< Rectangle that contains the extent (bounding box) of the layer
@@ -339,6 +335,7 @@ class QgsOracleProvider final: public QgsVectorDataProvider
 
     bool mHasSpatialIndex;                   //!< Geometry column is indexed
     QString mSpatialIndexName;               //!< Name of spatial index of geometry column
+    int mOracleVersion;                      //!< Oracle database version
 
     std::shared_ptr<QgsOracleSharedData> mShared;
 
@@ -398,6 +395,8 @@ class QgsOracleSharedData
 
 class QgsOracleProviderMetadata final: public QgsProviderMetadata
 {
+    Q_OBJECT
+
   public:
     QgsOracleProviderMetadata();
     QString getStyleById( const QString &uri, QString styleId, QString &errCause ) override;

diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py
@@ -32,7 +32,7 @@
     QgsFieldConstraints,
     NULL
 )
-from qgis.PyQt.QtCore import QDate, QTime, QDateTime
+from qgis.PyQt.QtCore import QDate, QTime, QDateTime, QVariant
 from qgis.PyQt.QtTest import QSignalSpy
 
 from utilities import compareWkt
@@ -1150,3 +1150,59 @@ def testEmptySubsetOfAttributesWithSubsetString(self):
 
             finally:
                 self.source.setSubsetString(None)
+
+    def testGeneratedColumns(self):
+
+        if not getattr(self, 'getGeneratedColumnsData', None):
+            return
+
+        vl, generated_value = self.getGeneratedColumnsData()
+        if vl is None:
+            return
+
+        self.assertTrue(vl.isValid())
+        self.assertEqual(vl.fields().count(), 2)
+
+        field = vl.fields().at(1)
+        self.assertEqual(field.name(), "generated_field")
+        self.assertEqual(field.type(), QVariant.String)
+        self.assertEqual(vl.dataProvider().defaultValueClause(1), generated_value)
+
+        vl.startEditing()
+
+        feature = next(vl.getFeatures())
+        # to be fixed
+        # self.assertEqual(QgsVectorLayerUtils.fieldIsEditable(vl, 1, feature), editable)
+
+        # same test on a new inserted feature
+        feature = QgsFeature(vl.fields())
+        feature.setAttribute(0, 2)
+        vl.addFeature(feature)
+        self.assertTrue(feature.id() < 0)
+        # to be fixed
+        # self.assertEqual(QgsVectorLayerUtils.fieldIsEditable(vl, 1, feature), editable)
+        vl.commitChanges()
+
+        feature = vl.getFeature(2)
+        self.assertTrue(feature.isValid())
+        self.assertEqual(feature.attribute(1), "test:2")
+
+        # to be fixed
+        # self.assertEqual(QgsVectorLayerUtils.fieldIsEditable(vl, 1, feature), editable)
+
+        # test update id and commit
+        vl.startEditing()
+        self.assertTrue(vl.changeAttributeValue(2, 0, 10))
+        self.assertTrue(vl.commitChanges())
+        feature = vl.getFeature(10)
+        self.assertTrue(feature.isValid())
+        self.assertEqual(feature.attribute(0), 10)
+        self.assertEqual(feature.attribute(1), "test:10")
+
+        # test update the_field and commit (the value is not changed because the field is generated)
+        vl.startEditing()
+        self.assertTrue(vl.changeAttributeValue(10, 1, "new value"))
+        self.assertTrue(vl.commitChanges())
+        feature = vl.getFeature(10)
+        self.assertTrue(feature.isValid())
+        self.assertEqual(feature.attribute(1), "test:10")
diff --git a/tests/src/python/test_provider_oracle.py b/tests/src/python/test_provider_oracle.py
@@ -770,6 +770,13 @@ def testDeterminePKOnView(self):
 
         self.assertEqual(vl.dataProvider().pkAttributeIndexes(), [0, 2])
 
+    def getGeneratedColumnsData(self):
+        """
+        return a tuple with the generated column test layer and the expected generated value
+        """
+        return (QgsVectorLayer(self.dbconn + ' sslmode=disable table="QGIS"."GENERATED_COLUMNS"', 'test', 'oracle'),
+                """'test:'||TO_CHAR("pk")""")
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py
@@ -138,6 +138,21 @@ def uncompiledFilters(self):
     def partiallyCompiledFilters(self):
         return set([])
 
+    def getGeneratedColumnsData(self):
+        """
+        return a tuple with the generated column test layer and the expected generated value
+        """
+        cur = self.con.cursor()
+        cur.execute("SHOW server_version_num")
+        pgversion = int(cur.fetchone()[0])
+
+        # don't trigger this test when PostgreSQL versions earlier than 12.
+        if pgversion < 120000:
+            return (None, None)
+        else:
+            return (QgsVectorLayer(self.dbconn + ' sslmode=disable table="qgis_test"."generated_columns"', 'test', 'postgres'),
+                    """('test:'::text || ((pk)::character varying)::text)""")
+
     # HERE GO THE PROVIDER SPECIFIC TESTS
     def testDefaultValue(self):
         self.source.setProviderProperty(

diff --git a/tests/testdata/provider/testdata_oracle.sql b/tests/testdata/provider/testdata_oracle.sql
@@ -72,4 +72,8 @@ CREATE TABLE QGIS.POINT_DATA_IDENTITY ( "pk" NUMBER GENERATED ALWAYS AS IDENTITY
 INSERT INTO QGIS.POINT_DATA_IDENTITY (GEOM)
  SELECT SDO_GEOMETRY( 2001,4326,SDO_POINT_TYPE(1, 2, NULL), NULL, NULL) from dual;
 
+CREATE TABLE QGIS.GENERATED_COLUMNS ( "pk" INTEGER PRIMARY KEY, "generated_field" GENERATED ALWAYS AS ('test:' || "pk") VIRTUAL);
+INSERT INTO QGIS.GENERATED_COLUMNS ("pk") SELECT 1 FROM dual;
+
+
 COMMIT;
diff --git a/tests/testdata/provider/testdata_pg_12_generated.sql b/tests/testdata/provider/testdata_pg_12_generated.sql
@@ -16,3 +16,7 @@ CREATE TABLE qgis_test.test_gen_geog_col (
   poly_area FLOAT NOT NULL GENERATED ALWAYS AS ( st_area(geog) ) STORED
 );
 
+CREATE TABLE qgis_test.generated_columns(
+  pk INTEGER PRIMARY KEY,
+  generated_field varchar(30) GENERATED ALWAYS AS ('test:' || "pk"::varchar) STORED);
+INSERT INTO qgis_test.generated_columns ("pk") VALUES(1);