Trust layer metadata propagation #38464

rldhont · 2020-08-26T15:57:13Z

Description

The trust flag at the project level is only used to read vector layer extent from xml, not from provider.

This flag was not available at the vector layer and data provider level.

We propose a new QgsMapLayer reading flag to propagate the trust layer metadata prohect's read flag and a new provider flag to trust datasource config.

Trusting the datasource config means that the provider can use estimated metadata, the primary key is unique and the detectable geometry type and srid are the same as the requested.

rldhont · 2020-08-26T16:00:36Z

@mdouchin @dmarteau Can you take a look at the code ? thanks

nyalldawson · 2020-08-26T23:21:33Z

src/core/qgsdataprovider.h

@@ -104,6 +104,7 @@ class CORE_EXPORT QgsDataProvider : public QObject
    struct ProviderOptions
    {
      QgsCoordinateTransformContext transformContext;
+      bool trustDatasourceConfig; //!< Trust datasource config (primary key unicity, geometry type and srid, etc). Improves provider load time.


It'd be more future-friendly if we also used a new flags setup here instead of bools. (Don't forget the "since" annotation too!)

I have removed my changes on struct ProviderOptions and added ReadFlags

nyalldawson · 2020-08-26T23:22:18Z

src/core/qgsmaplayer.h

@@ -553,6 +553,7 @@ class CORE_EXPORT QgsMapLayer : public QObject
    enum ReadFlag
    {
      FlagDontResolveLayers = 1 << 0, //!< Don't resolve layer paths or create data providers for layers.
+      FlagTrustLayerMetadata = 1 << 1, //!< Trust layer metadata. Improves layer load time.


Suggested change

FlagTrustLayerMetadata = 1 << 1, //!< Trust layer metadata. Improves layer load time.

FlagTrustLayerMetadata = 1 << 1, //!< Trust layer metadata. Improves layer load time by skipping expensive checks like ######### on layer load. Since QGIS 3.16

nyalldawson · 2020-08-26T23:24:28Z

src/core/raster/qgsrasterlayer.cpp

@@ -125,7 +125,7 @@ QgsRasterLayer::QgsRasterLayer( const QString &uri,
  QgsDebugMsgLevel( QStringLiteral( "Entered" ), 4 );
  setProviderType( providerKey );

-  QgsDataProvider::ProviderOptions providerOptions { options.transformContext };
+  QgsDataProvider::ProviderOptions providerOptions { options.transformContext, ( mReadFlags & QgsMapLayer::FlagTrustLayerMetadata ) ? true : false };


This is a fragile way of constructing ProviderOptions -- if someone reorganised the members in the future then this code would break. Better to make an explicit constructor for ProviderOptions so that this is using stable api.

I have removed my changes on struct ProviderOptions and added ReadFlags

nyalldawson · 2020-08-26T23:25:59Z

src/core/qgsdataprovider.h

@@ -628,18 +629,18 @@ class CORE_EXPORT QgsDataProvider : public QObject
    //! Sets error message
    void setError( const QgsError &error ) { mError = error;}

-  private:


This change doesn't seem required -- can you revert?

I have moved mOptions from private to protected because build stopped with the message

../src/providers/postgres/qgspostgresprovider.cpp:159:58: error: 'mOptions' is a private member of 'QgsDataProvider' mUseEstimatedMetadata = mUri.useEstimatedMetadata() || mOptions.trustDatasourceConfig; ^ ../src/core/qgsdataprovider.h:642:38: note: declared private here QgsDataProvider::ProviderOptions mOptions; ^ ../src/providers/postgres/qgspostgresprovider.cpp:1737:39: error: 'mOptions' is a private member of 'QgsDataProvider' if ( mCheckPrimaryKeyUnicity || mOptions.trustDatasourceConfig ) ^ ../src/core/qgsdataprovider.h:642:38: note: declared private here QgsDataProvider::ProviderOptions mOptions; ^ ../src/providers/postgres/qgspostgresprovider.cpp:3740:8: error: 'mOptions' is a private member of 'QgsDataProvider' if ( mOptions.trustDatasourceConfig ) ^ ../src/core/qgsdataprovider.h:642:38: note: declared private here QgsDataProvider::ProviderOptions mOptions; ^ 3 errors generated.

And I have moved mDataSourceURI from private to protected because build generates a warning:

[130/2497] Building CXX object src/core/CMakeFiles/qgis_core.dir/qgsdataprovider.cpp.o ../src/core/qgsdataprovider.cpp:23:5: warning: field 'mDataSourceURI' will be initialized after field 'mOptions' [-Wreorder] : mDataSourceURI( uri ), ^ 1 warning generated.

I have fixed it by created mReadFlags

src/providers/postgres/qgspostgresprovider.cpp

rldhont · 2020-09-08T22:40:03Z

Hi @nyalldawson, what do you thonk about ths flags implémentation ?

nyalldawson

Looks good to me, nice work!

The trust flag at the projetc level is only used to read vector layer extent from xml, not from provider. This flag was not available at the vector layer and data provider level. We propose a new QgsMapLayer reading flag to propagate the trust layer metadata prohect's read flag an d a new provider options to trust datasource config. Trusting the datasource config means that the provider can use estimated metadata, the primary key is unique and the detectable geometry type and srid are the same as the requested.

rldhont · 2020-09-10T15:42:17Z

Thanks @nyalldawson

rldhont · 2020-09-11T06:50:47Z

@pblottiere or @elpaso do you agree to merge ?

rldhont · 2020-09-11T07:23:35Z

tahnsk @pblottiere @elpaso -> Speed up QGIS Server ;-)

pblottiere · 2020-09-11T07:25:21Z

tahnsk @pblottiere @elpaso -> Speed up QGIS Server ;-)

Thanks to you @rldhont, very good work 👍

rldhont added Feature API API improvement only, no visible user interface changes Data Provider Related to specific vector, raster or mesh data providers labels Aug 26, 2020

rldhont added this to the 3.16 (Feature) milestone Aug 26, 2020

rldhont requested review from elpaso, nyalldawson and pblottiere August 26, 2020 15:57

This was referenced Aug 26, 2020

New project reading flag to trust layer metadata with server settings #38225

Merged

Improve QGIS project loading time - mainly for QGIS Server qgis/QGIS-Enhancement-Proposals#191

Open

rldhont force-pushed the trust-layer-metadata-propagation branch from debeb68 to 4b9f36d Compare August 26, 2020 16:41

nyalldawson reviewed Aug 26, 2020

View reviewed changes

rldhont force-pushed the trust-layer-metadata-propagation branch from af75ee1 to 191f04e Compare August 27, 2020 12:03

zacharlie added the Changelog Items that are queued to appear in the visual changelog - remove after harvesting label Aug 27, 2020

mdouchin reviewed Aug 28, 2020

View reviewed changes

src/providers/postgres/qgspostgresprovider.cpp Outdated Show resolved Hide resolved

rldhont commented Aug 28, 2020

View reviewed changes

src/providers/postgres/qgspostgresprovider.cpp Outdated Show resolved Hide resolved

rldhont force-pushed the trust-layer-metadata-propagation branch 2 times, most recently from df5e221 to a048bce Compare September 1, 2020 09:19

rldhont force-pushed the trust-layer-metadata-propagation branch 2 times, most recently from 7dc27ea to 8d4625c Compare September 8, 2020 09:30

nyalldawson approved these changes Sep 9, 2020

View reviewed changes

rldhont added 4 commits September 9, 2020 09:35

Postgres and Oracle: Use trust datasource config flag

760347c

MSSQL and DB2: Use trust datasource config flag

74ffc1a

Update PyQgsPythonProvider test

02cff9f

rldhont force-pushed the trust-layer-metadata-propagation branch from e09e7ae to 02cff9f Compare September 9, 2020 07:48

pblottiere approved these changes Sep 11, 2020

View reviewed changes

elpaso merged commit 63d8ee7 into qgis:master Sep 11, 2020

rldhont mentioned this pull request Sep 11, 2020

Raster layer can use XML extent like vector layer #38662

Closed

zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels Sep 12, 2020

rldhont mentioned this pull request Sep 17, 2020

Fix PG trust metadata layer loading #38833

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trust layer metadata propagation #38464

Trust layer metadata propagation #38464

rldhont commented Aug 26, 2020 •

edited

Loading

rldhont commented Aug 26, 2020

nyalldawson Aug 26, 2020

rldhont Sep 7, 2020

nyalldawson Aug 26, 2020

nyalldawson Aug 26, 2020

rldhont Sep 7, 2020

nyalldawson Aug 26, 2020

rldhont Aug 27, 2020

rldhont Sep 7, 2020

rldhont commented Sep 8, 2020

nyalldawson left a comment

rldhont commented Sep 10, 2020

rldhont commented Sep 11, 2020

rldhont commented Sep 11, 2020

pblottiere commented Sep 11, 2020

	FlagTrustLayerMetadata = 1 << 1, //!< Trust layer metadata. Improves layer load time.
	FlagTrustLayerMetadata = 1 << 1, //!< Trust layer metadata. Improves layer load time by skipping expensive checks like ######### on layer load. Since QGIS 3.16

Trust layer metadata propagation #38464

Trust layer metadata propagation #38464

Conversation

rldhont commented Aug 26, 2020 • edited Loading

Description

rldhont commented Aug 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rldhont commented Sep 8, 2020

nyalldawson left a comment

Choose a reason for hiding this comment

rldhont commented Sep 10, 2020

rldhont commented Sep 11, 2020

rldhont commented Sep 11, 2020

pblottiere commented Sep 11, 2020

rldhont commented Aug 26, 2020 •

edited

Loading