Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FEATURE] Trust project option #5094

Merged
merged 13 commits into from
Sep 7, 2017
Merged

[FEATURE] Trust project option #5094

merged 13 commits into from
Sep 7, 2017

Conversation

pblottiere
Copy link
Member

@pblottiere pblottiere commented Aug 30, 2017 •
edited
Loading

Description

This PR adds the trustProject option.

When this option is activated:

  • extent for vector layers is directly read from the project file when the data provider has no metadata (for example in case of PostgreSQL views or materialized views)
  • primary key unicity is not checked in the PostgreSQL data provider

This option is convenient for the server in order to avoid spending to much time in getCapabilities when we know that the underlying data do not change.

Some tests has been added to confirm the behavior on PostgreSQL views and materialized views.

Checklist

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include fixes #11111 in the commit message next to the description
  • Commits which add new features are tagged with [FEATURE] in the commit message
  • Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and containt sufficient information in the commit message to be documented
  • I have read the QGIS Coding Standards and this PR complies with them
  • This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit

@haubourg
Copy link
Member

haubourg commented Aug 30, 2017 •
edited
Loading

Note: this option will greatly speed up project loading either on Desktop or Server Context for big datasets accessed via views, materialized views or any data format not storing such metadata. Downsides are that if datasource has effectively changed, Project will need to be updated

@nyalldawson
Copy link
Collaborator

Shouldn't this be set in the project properties rather than as a global option?

@pblottiere
Copy link
Member Author

Shouldn't this be set in the project properties rather than as a global option?

Good point... @haubourg do you agree?

@haubourg
Copy link
Member

+1 !

nyalldawson
nyalldawson reviewed Sep 4, 2017
View reviewed changes
src/core/qgsproject.h Outdated
*
* \since QGIS 3.0
*/
void setTrust( bool trust );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setTrustLayerMetadata/trustLayerMetadata would be more descriptive names here

nyalldawson
nyalldawson reviewed Sep 4, 2017
View reviewed changes
src/ui/qgsoptionsbase.ui Outdated
@@ -2005,7 +1951,7 @@
<item>
<widget class="QgsCollapsibleGroupBox" name="mSimplifyDrawingGroupBox">
<property name="title">
<string>Enable feature si&amp;mplification by default for newly added layers</string>
<string>Enable fea&amp;ture simplification by default for newly added layers</string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these shortcut changes deliberate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely not, my qt-designer made something strange here...

Copy link
Member Author

@pblottiere pblottiere Sep 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As soon as I open/save this file with my Qt-Designer, shortcuts are changed... Anyway, as trustProject is not a global option anymore, I reverted changes on qgsoptionsbase.ui.

@pblottiere
Copy link
Member Author

I'll merge this PR tomorrow if I've no more comments in the meantime.

@haubourg
Copy link
Member

haubourg commented Sep 6, 2017 •
edited
Loading

In project properties, the option would may be better placed in the "Data Sources" tab.
BTW, renaming "Data Sources" to "Data sources" would be nice :)

@haubourg
Copy link
Member

haubourg commented Sep 6, 2017

Beyond that, I couldn't find any problem in editing / updating extent workflow.
Editing from QGIS updates it automatically which is good. (Maybe with 1 trillion rows it would be nice to skip that too, let's see when his happens).
Changing data from the DB, QGIS with trust option will not change the extent unless pressing the "update extent button".
So +1 for me!

@pblottiere
Copy link
Member Author

In project properties, the option would may be better placed in the "Data Sources" tab.

Done

@pblottiere pblottiere merged commit 20d8244 into qgis:master Sep 7, 2017
@rldhont rldhont added the Server label Nov 6, 2017
@rldhont rldhont added this to the QGIS 3 milestone Nov 6, 2017
rldhont added a commit to rldhont/ms_perfs that referenced this pull request Jan 5, 2018
@rldhont
Test the Trust project option
c7ce119 
In QGIS3, the user can add an option to force to trust the project about extent and primarykey in PostgreSQL/Postgis layers
qgis/QGIS#5094
@rldhont rldhont mentioned this pull request Jan 5, 2018
Merged
@troopa81 troopa81 mentioned this pull request Mar 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nyalldawson nyalldawson nyalldawson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0.x
Development

Successfully merging this pull request may close these issues.
4 participants
@pblottiere @haubourg @nyalldawson @rldhont