Re: [QGIS-Developer] Data provider default values and default value clause
On Mon, 27 Jan 2020 at 18:02, Alessandro Pasotti wrote: > > Hi Nyall, > > thank you for confirming my worst nightmares :) > > Unfortunately, since the data providers are certainly a foundation component > of QGIS, these behavior inconsistencies lead to issues in client code (GUI/UX > mainly but probably also in QgsVectorLayerUtils create features functions) > and I believe we should try hard to find a solution. Well - I realise it's nitpicking, but I don't see this as a bug, rather as a feature request. Before the default value/ default value clause api was introduced we had NO way of differentiating these two, for any providers. We'd treat everything as a clause and there was lots of bugs as a result. When it was introduced the majority of providers were updated to utilise it, with the expectation of those which **couldn't** (due to backend limitations). For those providers there was no regressions -- they just didn't get the improved handling/user experience that the other providers gained. It's technically a "feature request" to somehow upgrade the remainder and find ways to differentiate default clauses from literal values for those providers. There may well be implementation bugs here (there always is!), but I think the existing API is fine and gives a sufficient level of distinction between the two. > Do you think it is a wast of time (because as you mentioned some providers > can't tell the difference between a literal and an expression)? ...Possibly? When I added them I did look into these backends with no luck, but you may have better luck then I did! (or just run into the same dead-ends...) Nyall > > I've done a PR that does not completely fix all the issues but at least adds > some tests and fixes a few things: https://github.com/qgis/QGIS/pull/34012 > > > > On Mon, Jan 27, 2020 at 7:44 AM Nyall Dawson wrote: >> >> On Fri, 24 Jan 2020 at 19:28, Alessandro Pasotti wrote: >> > >> >> > 1. From the docs it seems that the defaultValueClause() should ONLY return >> > clauses (like sequences, functions, CURRENT_TIMESTAMP etc.) and should NOT >> > return literal defaults. >> >> Correct. However -- not all providers are completely well behaved in >> this regard. Some of these are because of limitations on the provider >> itself, e.g. there is no way to different literal defaults from >> clause-type defaults on the associated backend. When we can't >> differentiate the two, we err on the side of returning a >> possible-literal value from defaultValueClause (and not the reverse >> and accidentally return a non-literal value from defaultValue() ). >> >> > 2. From the docs it seems that defaultValue() should return ONLY literal >> > defaults and NOT functions, ::nextval and friends >> >> Correct (and should ALWAYS be the case) >> >> > 3. OGR provider does return the actual client-side calculated value when >> > calling defaultValue() ONLY in case of literal defaults and >> > CURRENT_TIMESTAMP, CURRENT_DATE and CURRENT_TIME, it returns the clause >> > definition in all other cases, is this correct? >> >> Yes - it's deliberate, even thought it slightly violates the above >> answer to (2) in that for this provider we pre-calculate these >> quasi-literal values on the qgis side. >> >> > 4. postgres provider in case the property EvaluateDefaultValues is true >> > does something more and send the clause to the server for evaluation, >> > returning the calculated value, otherwise it returns the clause definition. >> >> That's correct also, and by design. As per the warnings in the >> defaultValue() docs, defaultValue calls are potentially dangerous to >> make and should only ever be done when creating features (and in that >> situation, only ever called once per field per feature). Otherwise >> when EvaluateDefaultValues is true we run the risk of evaluating >> sequences multiple times for a single feature, causing "gaps" in the >> sequence. >> >> > 5. What to return in case of no defaults? Depending on provider and field >> > types, some implementations return a NULL (QVariant()), some others return >> > a Python None. >> >> Slight correction: QVariant() != Python NULL. Rather QVariant() == >> Python None, and QVariant( QVariant::Int ) == Python NULL. The c++ api >> doesn't usually differentiate between the two and QVariant() is more >> commonly used. (And I personally think in 4.0 we should completely >> remove the PyQGIS NULL/QVariant( QVariant::Int ) distinctions -- they >> add much complexity to code without compelling enough benefits). >> >> Short answer: return QVariant() when no default literal exists. >> >> > So, I'm confused about the expected behavior, if the documentation of >> > defaultValue() and defaultValueClause() is correct then the provider >> > implementations are (at least for some of them) wrong. >> >> Hope that clarifies! >> >> Nyall >> >> > >> > I'd like to hear other developer's opinion before proceeding with a fix >> > for the a.m. providers. >> > >> > Thank you
Re: [QGIS-Developer] Data provider default values and default value clause
Hi Nyall, thank you for confirming my worst nightmares :) Unfortunately, since the data providers are certainly a foundation component of QGIS, these behavior inconsistencies lead to issues in client code (GUI/UX mainly but probably also in QgsVectorLayerUtils create features functions) and I believe we should try hard to find a solution. Do you think it is a wast of time (because as you mentioned some providers can't tell the difference between a literal and an expression)? I've done a PR that does not completely fix all the issues but at least adds some tests and fixes a few things: https://github.com/qgis/QGIS/pull/34012 On Mon, Jan 27, 2020 at 7:44 AM Nyall Dawson wrote: > On Fri, 24 Jan 2020 at 19:28, Alessandro Pasotti > wrote: > > > > > 1. From the docs it seems that the defaultValueClause() should ONLY > return clauses (like sequences, functions, CURRENT_TIMESTAMP etc.) and > should NOT return literal defaults. > > Correct. However -- not all providers are completely well behaved in > this regard. Some of these are because of limitations on the provider > itself, e.g. there is no way to different literal defaults from > clause-type defaults on the associated backend. When we can't > differentiate the two, we err on the side of returning a > possible-literal value from defaultValueClause (and not the reverse > and accidentally return a non-literal value from defaultValue() ). > > > 2. From the docs it seems that defaultValue() should return ONLY literal > defaults and NOT functions, ::nextval and friends > > Correct (and should ALWAYS be the case) > > > 3. OGR provider does return the actual client-side calculated value when > calling defaultValue() ONLY in case of literal defaults and > CURRENT_TIMESTAMP, CURRENT_DATE and CURRENT_TIME, it returns the clause > definition in all other cases, is this correct? > > Yes - it's deliberate, even thought it slightly violates the above > answer to (2) in that for this provider we pre-calculate these > quasi-literal values on the qgis side. > > > 4. postgres provider in case the property EvaluateDefaultValues is true > does something more and send the clause to the server for evaluation, > returning the calculated value, otherwise it returns the clause definition. > > That's correct also, and by design. As per the warnings in the > defaultValue() docs, defaultValue calls are potentially dangerous to > make and should only ever be done when creating features (and in that > situation, only ever called once per field per feature). Otherwise > when EvaluateDefaultValues is true we run the risk of evaluating > sequences multiple times for a single feature, causing "gaps" in the > sequence. > > > 5. What to return in case of no defaults? Depending on provider and > field types, some implementations return a NULL (QVariant()), some others > return a Python None. > > Slight correction: QVariant() != Python NULL. Rather QVariant() == > Python None, and QVariant( QVariant::Int ) == Python NULL. The c++ api > doesn't usually differentiate between the two and QVariant() is more > commonly used. (And I personally think in 4.0 we should completely > remove the PyQGIS NULL/QVariant( QVariant::Int ) distinctions -- they > add much complexity to code without compelling enough benefits). > > Short answer: return QVariant() when no default literal exists. > > > So, I'm confused about the expected behavior, if the documentation of > defaultValue() and defaultValueClause() is correct then the provider > implementations are (at least for some of them) wrong. > > Hope that clarifies! > > Nyall > > > > > I'd like to hear other developer's opinion before proceeding with a fix > for the a.m. providers. > > > > Thank you in advance! > > > > -- > > Alessandro Pasotti > > w3: www.itopen.it > > ___ > > QGIS-Developer mailing list > > QGIS-Developer@lists.osgeo.org > > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer > > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer > -- Alessandro Pasotti w3: www.itopen.it ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] Data provider default values and default value clause
On Fri, 24 Jan 2020 at 19:28, Alessandro Pasotti wrote: > > 1. From the docs it seems that the defaultValueClause() should ONLY return > clauses (like sequences, functions, CURRENT_TIMESTAMP etc.) and should NOT > return literal defaults. Correct. However -- not all providers are completely well behaved in this regard. Some of these are because of limitations on the provider itself, e.g. there is no way to different literal defaults from clause-type defaults on the associated backend. When we can't differentiate the two, we err on the side of returning a possible-literal value from defaultValueClause (and not the reverse and accidentally return a non-literal value from defaultValue() ). > 2. From the docs it seems that defaultValue() should return ONLY literal > defaults and NOT functions, ::nextval and friends Correct (and should ALWAYS be the case) > 3. OGR provider does return the actual client-side calculated value when > calling defaultValue() ONLY in case of literal defaults and > CURRENT_TIMESTAMP, CURRENT_DATE and CURRENT_TIME, it returns the clause > definition in all other cases, is this correct? Yes - it's deliberate, even thought it slightly violates the above answer to (2) in that for this provider we pre-calculate these quasi-literal values on the qgis side. > 4. postgres provider in case the property EvaluateDefaultValues is true does > something more and send the clause to the server for evaluation, returning > the calculated value, otherwise it returns the clause definition. That's correct also, and by design. As per the warnings in the defaultValue() docs, defaultValue calls are potentially dangerous to make and should only ever be done when creating features (and in that situation, only ever called once per field per feature). Otherwise when EvaluateDefaultValues is true we run the risk of evaluating sequences multiple times for a single feature, causing "gaps" in the sequence. > 5. What to return in case of no defaults? Depending on provider and field > types, some implementations return a NULL (QVariant()), some others return a > Python None. Slight correction: QVariant() != Python NULL. Rather QVariant() == Python None, and QVariant( QVariant::Int ) == Python NULL. The c++ api doesn't usually differentiate between the two and QVariant() is more commonly used. (And I personally think in 4.0 we should completely remove the PyQGIS NULL/QVariant( QVariant::Int ) distinctions -- they add much complexity to code without compelling enough benefits). Short answer: return QVariant() when no default literal exists. > So, I'm confused about the expected behavior, if the documentation of > defaultValue() and defaultValueClause() is correct then the provider > implementations are (at least for some of them) wrong. Hope that clarifies! Nyall > > I'd like to hear other developer's opinion before proceeding with a fix for > the a.m. providers. > > Thank you in advance! > > -- > Alessandro Pasotti > w3: www.itopen.it > ___ > QGIS-Developer mailing list > QGIS-Developer@lists.osgeo.org > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
[QGIS-Developer] Data provider default values and default value clause
Hi, Sorry for the long post, but I need some advice on the current API: I'm working on a bugfix (https://github.com/qgis/QGIS/issues/33383) and (as sometimes happens) I just opened another can of worms, I'm currently testing the following providers: - postgres - spatialite - ogr (spatialite opened with ogr provider) and the following methods (dp is the QgsDataProvider instance): dp.defaultValue(fieldIndex) dp.defaultValueClause(fieldIndex) the behavior of the tested providers is totally different and before attempting to homogenize them, I'd like to be sure about my understanding of the expected behavior, the relevant docs are: https://qgis.org/api/classQgsVectorDataProvider.html#ad2a51ae0b1928a179e258e7eba6d94a7 https://qgis.org/api/classQgsVectorDataProvider.html#a8b8b1fe945ec289885d77f820c85f8b6 Note that in case of a literal default value (like "123", "My default text") the result of the two function calls in some (but not all) of the current provider implementation may be the same (except for the returned type). Questions: 1. From the docs it seems that the defaultValueClause() should ONLY return clauses (like sequences, functions, CURRENT_TIMESTAMP etc.) and should NOT return literal defaults. 2. From the docs it seems that defaultValue() should return ONLY literal defaults and NOT functions, ::nextval and friends 3. OGR provider does return the actual client-side calculated value when calling defaultValue() ONLY in case of literal defaults and CURRENT_TIMESTAMP, CURRENT_DATE and CURRENT_TIME, it returns the clause definition in all other cases, is this correct? 4. postgres provider in case the property EvaluateDefaultValues is true does something more and send the clause to the server for evaluation, returning the calculated value, otherwise it returns the clause definition. 5. What to return in case of no defaults? Depending on provider and field types, some implementations return a NULL (QVariant()), some others return a Python None. So, I'm confused about the expected behavior, if the documentation of defaultValue() and defaultValueClause() is correct then the provider implementations are (at least for some of them) wrong. I'd like to hear other developer's opinion before proceeding with a fix for the a.m. providers. Thank you in advance! -- Alessandro Pasotti w3: www.itopen.it ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
