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.

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

filter expression error returns true #34259

Closed
mathieubossaert opened this issue Feb 4, 2020 · 16 comments · Fixed by #34309 or #34368
Closed

filter expression error returns true #34259

mathieubossaert opened this issue Feb 4, 2020 · 16 comments · Fixed by #34309 or #34368
Assignees
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Expressions Related to the QGIS expression engine or specific expression functions

Comments

@mathieubossaert
Copy link

Hi,

teaching QGIS on 3.4.15 today, we face a bug on MAC and windows (have to try on ubuntu) :
When we create a filter on a layer, if the expression contains an error ( for example a parenthesis not closed), the expression returns true so the layer is not filtered.
filter_error_returns_true

@mathieubossaert mathieubossaert added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Feb 4, 2020
@gioman
Copy link
Contributor

gioman commented Feb 4, 2020

@mathieubossaert I cannot confirm on 3.10.2 on Ubuntu. Can you attach a sample dataset?

@gioman gioman added the Feedback Waiting on the submitter for answers label Feb 4, 2020
@mathieubossaert
Copy link
Author

mathieubossaert commented Feb 4, 2020

Thanks @gioman
You can try this filter :

"STATUT" = 'Préfecture de région'sdqsdqsdqsdqs

On that shapefile :
communes_occitanie.zip

By the way if this bug does not exist in 3.10 we will upgrade.

@gioman
Copy link
Contributor

gioman commented Feb 4, 2020

By the way if this bug does not exist in 3.10 we will upgrade.

@mathieubossaert if is ok in 3.10 I believe than there will be no fix for 3.4 as 3.10 is becoming very soon the next LTR (not totally sure but there will not be a new 3.4 point release).

@mathieubossaert
Copy link
Author

ok Thanks. Let's close this issue.

@Gustry
Copy link
Contributor

Gustry commented Feb 4, 2020

@mathieubossaert I cannot confirm on 3.10.2 on Ubuntu. Can you attach a sample dataset?

Did you try with a shapefile ? I can replicate this issue with any shapefile on master. (3.11)

@gioman
Copy link
Contributor

gioman commented Feb 4, 2020

Did you try with a shapefile ?

@Gustry actually I think I tested with a postGIS layer, if is confirmed in 3.10/master let's reopen.

@gioman gioman reopened this Feb 4, 2020
@mathieubossaert
Copy link
Author

this will be the proof for my students of the utility to report issues ;-)
In fact I use PostGIS at work so i did not rise this issue before.

@Gustry
Copy link
Contributor

Gustry commented Feb 4, 2020

The feedback label can be removed ;-)

@gioman gioman removed the Feedback Waiting on the submitter for answers label Feb 4, 2020
@gioman
Copy link
Contributor

gioman commented Feb 4, 2020

The feedback label can be removed ;-)

removed

@pigreco
Copy link
Sponsor Contributor

pigreco commented Feb 4, 2020

I did tests on 3.4.15, 3.10.2-2 and masters

for the shared shapefile I confirm.

Using a geopackage I get (on all versions):

image

With vector spatialite I don't confirm(on all versions)

OSGeo4W64 win 10

@gioman gioman added the Expressions Related to the QGIS expression engine or specific expression functions label Feb 5, 2020
@elpaso elpaso self-assigned this Feb 5, 2020
elpaso added a commit to elpaso/QGIS that referenced this issue Feb 5, 2020
@uclaros
Copy link
Contributor

uclaros commented Feb 6, 2020

This only fixes half the bug, which is when the user presses the test button. One can still click OK with a broken WHERE clause and have all features returned while he should not be able to. The root problem is probably mLayer->setSubsetString( txtSQL->text() ) not returning false on broken queries.

@roya0045
Copy link
Contributor

roya0045 commented Feb 6, 2020

What @uclaros said, setSubsetString should return a bool. I guess that this is used to validate the expression and could be used to validate the expression instead of checking the count.

@gioman
Copy link
Contributor

gioman commented Feb 8, 2020

This only fixes half the bug, which is when the user presses the test button. One can still click OK with a broken WHERE clause and have all features returned while he should not be able to.

Should this be reopened or there is already a new ticket?

@elpaso
Copy link
Contributor

elpaso commented Feb 8, 2020

Feature request I would say. There's no validation for SQL in code.

@uclaros
Copy link
Contributor

uclaros commented Feb 8, 2020

It's a bug in QgsOgrProviderUtils and it's consequences are described in this issue. I've got a PR ready for it so I say just reopen so it can reclose!

@elpaso
Copy link
Contributor

elpaso commented Feb 8, 2020

Ok, I thought you were talking about validating the SQL syntax before sending it to the provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Expressions Related to the QGIS expression engine or specific expression functions
Projects
None yet
7 participants