Closed elpaso closed 2 years ago
I have strong reservations about this proposed approach, to the extent that I'm -1 to implementing in this way.
Here's a summary of my concerns:
First, keep in mind these constraints:
"field" = 'value'
, we don't (and can't) compile this directly for some providers (those which use case-insensitive comparisons on the backend, like SQL server). If we compile "field" = 'some string'
for SQL server we only get back a Partial compilation result, and we HAVE to filter all the results on QGIS side too to ensure that the comparisons are case sensitive.So, with these constraints in mind....
I really can't see how we can get a good user experience with the proposed approach. As you've (correctly) stated we can apply the QGIS expression as a provider filter "only if the expression can be fully compiled into a provider native filter". This will only be true for limited, and very unpredictable, filter strings. So taking the SQL server example a user will try to set a supposedly very simple filter of "field" = @some_variable
, and this WON'T be accepted. The user will have no real way of knowing why this (simple) filter isn't accepted, and will likely waste a bunch of time trying to modify their expression before ultimately giving up in frustration.
Even if the user knew enough about QGIS internals and the backend provider to realise that the filter can't be used because of the difference in QGIS/SQL Server handling of case sensitivity (and that's a HUGE ask), that same expert user will likely be completely stumped on why a $area > @some_variable
filter fails on Postgres, thinking that "postgres has ST_Area, so QGIS will be converting $area to ST_Area and it should work".
So let's say we try to address this issue by adding some detailed "explanation of compilation failure" handling to the expression compilation. And then we could show the user an error like "This QGIS expression can't be used as a filter string because SQL server doesn't support case-sensitive string comparisons". That's better, but still ultimately a frustration/dead end for the user... they'll leave thinking "well if it can't even support "field"='value', what's the point of this feature?".
In summary -- given that this approach will be usable in a very limited set of circumstances, I don't think it's appropriate for inclusion in QGIS core. The same approach could theoretically be exposed through a plugin which manages the expression compilation and updates when project/global variables change, so I'd say this is a better fit for a custom plugin instead.
@nyalldawson thanks for the feedback.
Are you also -1 on implementing var
function compilation in the SQL compilers? I don't see a problem there.
Are you also -1 on implementing var function compilation in the SQL compilers?
You possibly don't need to do that -- if you prepare the expression first using the context, then it will automatically replace any var nodes in the expression with static values which it can, and then it uses those during the compilation. See https://github.com/qgis/QGIS/pull/41494
QGIS Enhancement: Use QgsExpression to set a compiled subset string
Date 2022/03/30
Author Alessandro Pasotti (@elpaso)
Contact elpaso at itopen dot it
maintainer @elpaso
Version QGIS 3.26
Summary
Currently, a provider filter (aka subset string) can be set directly using the SQL editor.
It would be desirable to store in a QgsVectorLayer an expression (a QgsExpression) that can be pre-processed through the provider's QgsSqlExpressionCompiler in order to create a subset string with variable substitutions before the layer is created.
The use case is for a "template" project with many layers that needs to be filtered using variables, for example a project with a
@region
variable that automatically constructs all layers filters based on the value of@region
.One possible approach would be adding full support to QgsExpressions directly in the provider but this would bring several problems given the fact that only a limited set of QgsExpression functions and tokens can be actually compiled into provider filters, this could lead to very bad performances when the filtering needs to happen on the client in case of Partial or Fail compilation.
Hereby we propose a simpler approach where the provider code is not changed and the pre-processing of the filter expressions is done in client code (QgsMapLayer) that ultimately sets the subset string only if the expression can be fully compiled into a provider native filter.
The application will take care of resetting the filter expression and after a successful compilation set the subset string when a referenced variable changes.
The functionality will be exposed in the GUI adding an expression editor in the subset string widget that allows to optionally set the expression that will be used to generate the subset string.
Proposed Solution
The solution requires that
QgsSqlExpressionCompiler
is able to evaluate variables (var()
function), in order to do that we will pass aQgsExpressionContext
to the compile methods, note that this would be desireable in any case, even if the rest of this proposal is not implemented.in
QgsDataProvider
we will add a method to compile an expression:in
QgsVectorLayer
we add a method to set the subset string from an expression:The rest of the logic will be in app or gui and will reset the subset string of the layer that have an expression-based subset string that contains any changed variable.
Example(s)
Affected Files
Data providers and QgsMapLayer.
Performance Implications
None
Backwards Compatibility
None
Votes
(required)