Fix literal wildcards in text filters - #4609
Conversation
Escape LIKE metacharacters in user-entered text and cover percent, underscore, escape-character, and combined inputs against an OGR GeoPackage layer. Assisted-by: OpenAI Codex
9b8a549 to
0b9a6d0
Compare
|
@Withalion I exact-rebased this onto current master 4c38f41 (new head 0b9a6d0; range-diff is identical). astyle 3.4.13 leaves all changed files unchanged, git diff --check passes, and the SQLite ESCAPE matrix for %, , !, and % gives the expected literal-match counts. I could not run the full Qt/QGIS native binary on this host, and the repository workflows are still push-only for forks (#4624 is the draft that addresses this), so native CI remains a maintainer-side gate. Could you review when convenient? |
Withalion
left a comment
Withalion
left a comment
There was a problem hiding this comment.
You mentioned several times that you are not able to build the application on your host, what host do you have? The build infra is available for major OSes. If you haven't build the app how do you know it works? Have you run your tests?
| textValue.replace( QStringLiteral( "!" ), QStringLiteral( "!!" ) ); | ||
| textValue.replace( QStringLiteral( "%" ), QStringLiteral( "!%" ) ); | ||
| textValue.replace( QStringLiteral( "_" ), QStringLiteral( "!_" ) ); | ||
| expressionCopy.replace( QStringLiteral( "@@value@@" ), textValue ); | ||
| expressionCopy.append( QStringLiteral( " ESCAPE '!'" ) ); |
There was a problem hiding this comment.
Create new function in InputUtils which will do these steps and just call it from here. Use backslash as the escape character instead. Also are these all the wildcards you can get?
| mController->processFilters( filterValues ); | ||
|
|
||
| const QString expected = QStringLiteral( "(\"Condition\" LIKE '%great%')" ); | ||
| const QString expected = QStringLiteral( "(\"Condition\" LIKE '%great%' ESCAPE '!')" ); |
There was a problem hiding this comment.
we are not escaping anything here
| QTemporaryDir tempDir; | ||
| QVERIFY( tempDir.isValid() ); | ||
| const QString layerPath = tempDir.filePath( QStringLiteral( "roads.gpkg" ) ); | ||
| QVERIFY( QFile::copy( TestUtils::testDataDir() + QStringLiteral( "/filtering/roads.gpkg" ), layerPath ) ); | ||
|
|
||
| const QString fieldName = QStringLiteral( "Condition" ); | ||
| QgsVectorLayer *layer = new QgsVectorLayer( | ||
| layerPath + QStringLiteral( "|layername=roads" ), | ||
| QStringLiteral( "text-filter-wildcards" ), | ||
| QStringLiteral( "ogr" ) | ||
| ); | ||
| QVERIFY( layer ); | ||
| QVERIFY( layer->isValid() ); | ||
| QgsProject::instance()->addMapLayer( layer ); |
There was a problem hiding this comment.
What's wrong with the approach in other tests?
| void TestFilterController::testTextFilterWildcards_data() | ||
| { | ||
| QTest::addColumn<QString>( "searchValue" ); | ||
| QTest::addColumn<QString>( "expectedPattern" ); | ||
| QTest::addColumn<qlonglong>( "expectedCount" ); | ||
|
|
||
| QTest::newRow( "percent" ) << QStringLiteral( "%" ) << QStringLiteral( "%!%%" ) << 2LL; | ||
| QTest::newRow( "underscore" ) << QStringLiteral( "_" ) << QStringLiteral( "%!_%" ) << 2LL; | ||
| QTest::newRow( "escape-character" ) << QStringLiteral( "!" ) << QStringLiteral( "%!!%" ) << 1LL; | ||
| QTest::newRow( "combined-wildcards" ) << QStringLiteral( "%_" ) << QStringLiteral( "%!%!_%" ) << 1LL; | ||
| } |
There was a problem hiding this comment.
don't use this, just use the expected values inline
| const QStringList values = | ||
| { | ||
| QStringLiteral( "plain" ), | ||
| QStringLiteral( "percent%only" ), | ||
| QStringLiteral( "under_score" ), | ||
| QStringLiteral( "bang!mark" ), | ||
| QStringLiteral( "both%_chars" ) | ||
| }; |
There was a problem hiding this comment.
use values that could match values in condition
| for ( const QString &value : values ) | ||
| { | ||
| QVERIFY( TestUtils::addFeatureToLayer( layer, fieldName, value ) ); | ||
| } |
There was a problem hiding this comment.
don't add more features to the layer there already are some, use them
| QVERIFY( !filterId.isEmpty() ); | ||
|
|
||
| QVariantMap filterValues; | ||
| filterValues[filterId] = QVariantList{ searchValue }; |
There was a problem hiding this comment.
Apply the filter values one by one and check the results after each one
2 participants
Summary
!,%, and_before substituting text filter values intoLIKEexpressionsESCAPE '!'clause so%and_are matched literally while the surrounding%...%still performs substring matching%_inputRoot cause
Text filter values were quoted for SQL string safety, but SQL
LIKEmetacharacters were left unchanged. As a result,%matched any sequence and_matched any single character instead of the characters users typed.Closes #4517.
Validation
astylecheck on all changed C++/header filesgit diff --checkLIKE ... ESCAPE '!'patterns for%,_,!, and%_roads.gpkgthrough the OGR providerThe full Mergin Maps native test binary was not built locally because this host does not have the repository's Qt/QGIS/vcpkg toolchain.
AI assistance disclosure
OpenAI Codex assisted with issue triage, implementation, regression-test drafting, and validation. The contribution was reviewed against the repository policies and the generated SQL behavior was independently checked locally.