Fix mixed layer oracle, fix #32521 #34358

SebastienPeillet · 2020-02-07T17:54:31Z

Description

When Qgis loads Oracle database, it lists a row per geometry type in a table (yes, by design it think it could'nt be authorized to have several geometry types in a table... but it's oracle).

In the issue example, for the same table, there is two entries. One for polygon and one for multipolygon. But when user loads the polygon layer, all polygon geometries (included multipolygon) are loaded and the reverse is also true for the multipolygon layer.

I fixed it by filtering simple and multi geometries in the oracle connection. Doing this, i had to fix some functions in the provider :

featuresCount function, to count only the wanted geometries,
addFeatures function because a simple geometry was not cast into multi geometry when the loaded layer was the multi geometry one (the new feature disappeared when user commits the changes)

EDIT : after seeing troubles by only filter single and multi geometry, I also filtered geometry types that were loaded together before (like LineString and CircularString), see my message below

I added testFilterSimpleMultiGeom test, but I also had to fix some tests in consequence.

Could be backported to 3.10.

CheckList

Commit messages are descriptive and explain the rationale for changes.
Commits which fix bugs include Fixes #11111 at the bottom of the commit message. If this is your first pull request and you forgot to do this, write the same statement into this text field with the pull request description.
New unit tests have been added for relevant changes
You have run the scripts/prepare-commit.sh script (https://github.com/qgis/QGIS/blob/master/.github/CONTRIBUTING.md#contributing-to-qgis) before each commit.
If you didn't do this, you can also run ./scripts/astyle-all.sh from your source folder.
You have read the QGIS Coding Standards (https://docs.qgis.org/testing/en/docs/developers_guide/codingstandards.html) and this PR complies with them

… the same table, fix addFeatures function to cast simple geom to multi geom when the loaded layer has multi geom type

…Peillet/qgis into fix_mixed_layer_oracle

troopa81

I think there is a problem because there is only one detected geometry when it could have several (multipolygon and polygon for instance)

EDIT: forget about this one, I miss the second qry.next and so the detectedType will be Unknown, so it works

src/providers/oracle/qgsoracleprovider.cpp

This reverts commit 930f08f.

tomass · 2020-03-11T08:41:44Z

What about a use case where we do not care if it is a polygon or a multi-polygon? This PR would break it?

stale · 2020-03-26T03:07:14Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

stale · 2020-04-02T03:16:20Z

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

stale · 2020-07-27T13:37:40Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

stale · 2020-08-08T07:52:23Z

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

stale · 2020-08-29T08:50:06Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

stale · 2020-09-20T03:44:16Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

rldhont · 2020-09-25T07:23:31Z

Conflict to resolve

…Peillet/qgis into fix_mixed_layer_oracle

troopa81 · 2020-09-28T10:19:15Z

src/providers/oracle/qgsoracleconn.cpp

    case QgsWkbTypes::LineString:
    case QgsWkbTypes::LineString25D:
    case QgsWkbTypes::LineStringZ:
+      // LineString gtype ends with 2, sdo_interpretation must not be equal to 2 (CircularString) or 3 (Nurbs)


I think this filter will decrease performances, because it requests all interpretation from the table and then join on PK. It is worth trying on big dataset but I'm pretty sure this one should be better

SELECT "pk" FROM QGIS.LINE_DATA l WHERE mod(l."GEOM"."SDO_GTYPE",100) = 2 AND ( SELECT MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) FROM TABLE(l."GEOM"."SDO_ELEM_INFO") t ) not in (2,3);

We should also put this request in a QString and reuse it in all following filters with just parameter for sdo_gtype and in/not in 2,3/4

troopa81 · 2020-10-05T13:26:14Z

@SebastienPeillet Modifications looks fine to me. Did you manage to make some test on big dataset to check if there is no performance issues

SebastienPeillet · 2020-10-06T08:52:55Z

@troopa81 I tried this morning with a table that has 162k features.

Without our modification : 162k features (simple and multi polygon) loaded in ~32s

With our modification : 108k features (simple polygon) loaded in ~35s

SebastienPeillet · 2020-10-07T16:02:43Z

@troopa81

I did some test in Oracle to avoid QGIS interferences.

There is two requests that are been changed. I limited result to 100k lines to get

First request :

SELECT count(*) FROM "QGIS"."MIX_SIMPLE_MULTI_POLYGON" "FEATUREREQUEST" WHERE ( mod("FEATUREREQUEST"."GEOM".sdo_gtype,100) = 3);

  COUNT(*)
----------
    108390

Elapsed: 00:00:00.13

and with changes :

SELECT count(*) FROM "QGIS"."MIX_SIMPLE_MULTI_POLYGON" "FEATUREREQUEST" WHERE ( mod("FEATUREREQUEST"."GEOM".sdo_gtype,100) = 3 AND (SELECT MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) FROM TABLE("FEATUREREQUEST"."GEOM".SDO_ELEM_INFO) t) not in (2,4) OR GEOM IS NULL);

  COUNT(*)
----------
    108390

Elapsed: 00:00:00.67

Second request

The second request is more difficult to evaluate due to print output. On one hand I measure the request with the print, on the other I remove the print by setting SET AUTOTRACE TRACEONLY but I got the plan explanation instead...

With output print :

The original request :

SELECT "pk" FROM "QGIS"."MIX_SIMPLE_MULTI_POLYGON" "FEATUREREQUEST" WHERE ( mod("FEATUREREQUEST"."GEOM".sdo_gtype,100) = 3 );

with changes :

SELECT "pk" FROM "QGIS"."MIX_SIMPLE_MULTI_POLYGON" "FEATUREREQUEST" WHERE ( mod("FEATUREREQUEST"."GEOM".sdo_gtype,100) = 3 AND (SELECT MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) FROM TABLE("FEATUREREQUEST"."GEOM".SDO_ELEM_INFO) t) not in (2,4) OR GEOM IS NULL);

for both, execution time is between 3.75s and 4.22s, depends on print surely.

Without print but with explain

For original request the execution time is between 0.78s and 1.02s

For modified request the execution time is between 1.42s and 1.56s

stale · 2020-11-07T21:57:53Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

nyalldawson · 2020-12-15T01:23:18Z

Closing manually because stale-bot seems to have gone on holiday

BUG: fix loading of oracle table when there is different geom type in…

2764909

… the same table, fix addFeatures function to cast simple geom to multi geom when the loaded layer has multi geom type

SebastienPeillet changed the title ~~Fix mixed layer oracle~~ Fix mixed layer oracle, fix #32521 Feb 7, 2020

Merge master into fix_mixed_layer_oracle

f984b27

SebastienPeillet force-pushed the fix_mixed_layer_oracle branch from 20032b4 to f984b27 Compare February 8, 2020 00:19

Merge master to fix_mixed_layer_oracle

0f3dbde

SebastienPeillet closed this Feb 8, 2020

SebastienPeillet reopened this Feb 8, 2020

SebastienPeillet and others added 4 commits February 8, 2020 02:03

Merge branch 'master' into fix_mixed_layer_oracle

0e93752

Fix spell test

4a703c0

Merge branch 'fix_mixed_layer_oracle' of https://github.com/Sebastien…

4577c39

…Peillet/qgis into fix_mixed_layer_oracle

BUG: fix invalid layer result when Oracle version is under 12

930f08f

nyalldawson added Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers labels Feb 22, 2020

nyalldawson added this to the 3.14.0 milestone Feb 22, 2020

troopa81 requested changes Feb 26, 2020

View reviewed changes

speillet added 2 commits March 9, 2020 12:28

Revert "BUG: fix invalid layer result when Oracle version is under 12"

4fc0f34

This reverts commit 930f08f.

FIX: apply review comment

cc16f3e

troopa81 mentioned this pull request Mar 11, 2020

Skip re-querying geometry type for Oracle provider #34738

Closed

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Mar 26, 2020

stale bot closed this Apr 2, 2020

Jean-Roc mentioned this pull request May 12, 2020

[Oracle] Force multi geometry type at layer level #36364

Merged

speillet added 3 commits May 25, 2020 21:24

Add single geometry cast to multi geometry

26f0621

Geometry filter now avoid "arc in geodetic data" error

b531ddd

Better geom filter, fix test to fit with unmixed layer

ac78a19

SebastienPeillet reopened this May 26, 2020

stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label May 26, 2020

OMerge branch 'master' into fix_mixed_layer_oracle

a7e5fde

SebastienPeillet removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 13, 2020

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 27, 2020

stale bot closed this Aug 8, 2020

SebastienPeillet reopened this Aug 11, 2020

stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 11, 2020

Meibes mentioned this pull request Aug 21, 2020

TIN creation crash #30549

Closed

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 29, 2020

SebastienPeillet closed this Sep 1, 2020

SebastienPeillet reopened this Sep 1, 2020

stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 1, 2020

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 20, 2020

SebastienPeillet removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 24, 2020

speillet and others added 3 commits September 25, 2020 10:37

Add explanation on geometry filter

3a6c6da

Merge branch 'master' into fix_mixed_layer_oracle

5d04971

Merge branch 'fix_mixed_layer_oracle' of https://github.com/Sebastien…

dba2e3a

…Peillet/qgis into fix_mixed_layer_oracle

troopa81 requested changes Sep 28, 2020

View reviewed changes

optimize sql filter

e534890

troopa81 mentioned this pull request Oct 14, 2020

Multi geometry on oracle table #23975

Closed

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 7, 2020

nyalldawson closed this Dec 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix mixed layer oracle, fix #32521 #34358

Fix mixed layer oracle, fix #32521 #34358

SebastienPeillet commented Feb 7, 2020 •

edited

Loading

troopa81 left a comment •

edited

Loading

tomass commented Mar 11, 2020

stale bot commented Mar 26, 2020

stale bot commented Apr 2, 2020

stale bot commented Jul 27, 2020

stale bot commented Aug 8, 2020

stale bot commented Aug 29, 2020

stale bot commented Sep 20, 2020

rldhont commented Sep 25, 2020

troopa81 Sep 28, 2020

troopa81 commented Oct 5, 2020

SebastienPeillet commented Oct 6, 2020

SebastienPeillet commented Oct 7, 2020

stale bot commented Nov 7, 2020

nyalldawson commented Dec 15, 2020

Fix mixed layer oracle, fix #32521 #34358

Fix mixed layer oracle, fix #32521 #34358

Conversation

SebastienPeillet commented Feb 7, 2020 • edited Loading

Description

CheckList

troopa81 left a comment • edited Loading

Choose a reason for hiding this comment

tomass commented Mar 11, 2020

stale bot commented Mar 26, 2020

stale bot commented Apr 2, 2020

stale bot commented Jul 27, 2020

stale bot commented Aug 8, 2020

stale bot commented Aug 29, 2020

stale bot commented Sep 20, 2020

rldhont commented Sep 25, 2020

troopa81 Sep 28, 2020

Choose a reason for hiding this comment

troopa81 commented Oct 5, 2020

SebastienPeillet commented Oct 6, 2020

SebastienPeillet commented Oct 7, 2020

First request :

Second request

With output print :

Without print but with explain

stale bot commented Nov 7, 2020

nyalldawson commented Dec 15, 2020

SebastienPeillet commented Feb 7, 2020 •

edited

Loading

troopa81 left a comment •

edited

Loading