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

If a widget default value is setup as NULL then it's not working as it's overriden by database column default #51818

Open
2 tasks done
tudorbarascu opened this issue Feb 12, 2023 · 9 comments
Assignees
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Widgets

Comments

@tudorbarascu
Copy link
Member

tudorbarascu commented Feb 12, 2023

What is the bug or the crash?

If a QGIS column widget has a default 'NULL' but if on PostgreSQL database side the default is 10, adding a new feature it defaults on 10 instead of NULL like it's setup at the QGIS project level. The evaluate values on database side checkbox is unchecked. Am I missing something?

I tested with the value relation and text widgets.

If the default value in QGIS is setup to any other value (except NULL) then everything works as it should.

Steps to reproduce the issue

  1. CREATE the following postgresql table:

CREATE TABLE test (id serial PRIMARY KEY, value smallint DEFAULT 10);

  1. Add the table in a QGIS project.

  2. Set the value column widget to default to NULL like so:
    image

  3. Adding a new feature/record defaults on 10 instead of NULL.

Versions

Tested with QGIS 3.22.16 and 3.28.3.

<style type="text/css"> p, li { white-space: pre-wrap; } </style>
QGIS version 3.28.3-Firenze QGIS code revision 90ca828
Qt version 5.15.8
Python version 3.11.1
GDAL/OGR version 3.5.2
PROJ version 9.0.1
EPSG Registry database version v10.064 (2022-05-19)
GEOS version 3.11.0-CAPI-1.17.0
SQLite version 3.40.0
PDAL version 2.4.3
PostgreSQL client version unknown
SpatiaLite version 5.0.1
QWT version 6.1.5
QScintilla2 version 2.13.0
OS version Fedora Linux 37 (Thirty Seven)
       
Active Python plugins
slyr_community 4.0.6
QuickWKT 3.1
project_export_inspire 1.0.0
water_tools 1.0
plugin_reloader 0.9.3
water_topology_plugin 0.1
db_manager 0.1.20
processing 2.12.99
sagaprovider 2.12.99
grassprovider 2.12.99
MetaSearch 0.3.6
QGIS version 3.28.3-Firenze QGIS code revision [90ca828](https://github.com/qgis/QGIS/commit/90ca828d04) Qt version 5.15.8 Python version 3.11.1 GDAL/OGR version 3.5.2 PROJ version 9.0.1 EPSG Registry database version v10.064 (2022-05-19) GEOS version 3.11.0-CAPI-1.17.0 SQLite version 3.40.0 PDAL version 2.4.3 PostgreSQL client version unknown SpatiaLite version 5.0.1 QWT version 6.1.5 QScintilla2 version 2.13.0 OS version Fedora Linux 37 (Thirty Seven)

Active Python plugins
slyr_community
4.0.6
QuickWKT
3.1
project_export_inspire
1.0.0
water_tools
1.0
plugin_reloader
0.9.3
water_topology_plugin
0.1
db_manager
0.1.20
processing
2.12.99
sagaprovider
2.12.99
grassprovider
2.12.99
MetaSearch
0.3.6

Supported QGIS version

  • I'm running a supported QGIS version according to the roadmap.

New profile

  • I tried with a new QGIS profile

Additional context

No response

@tudorbarascu tudorbarascu added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Feb 12, 2023
@tudorbarascu tudorbarascu changed the title If a widget default value is setup as NULL that's overriden by database column default If a widget default value is setup as NULL it's overriden by database column default Feb 12, 2023
@tudorbarascu tudorbarascu changed the title If a widget default value is setup as NULL it's overriden by database column default If a widget default value is setup as NULL then itit's overriden by database column default Feb 21, 2023
@tudorbarascu tudorbarascu changed the title If a widget default value is setup as NULL then itit's overriden by database column default If a widget default value is setup as NULL then itt's overriden by database column default Feb 21, 2023
@tudorbarascu tudorbarascu changed the title If a widget default value is setup as NULL then itt's overriden by database column default If a widget default value is setup as NULL then it's not working as it's overriden by database column default Feb 21, 2023
@elpaso
Copy link
Contributor

elpaso commented Feb 21, 2023

Cannot reproduce on Linux master.

this is what is sent to the server:

PQprepare(addfeatures): INSERT INTO "public"."gh_51818"("value") VALUES (NULL) RETURNING "id"

@tudorbarascu
Copy link
Member Author

hmm, retested and I can reproduce on multiple systems: Fedora Arm (Master + 3.22.16), Debian x86 Bullseye (Master + 3.22.16) and Windows 10 QGIS - Master.
CREATE TABLE test (id serial PRIMARY KEY, value smallint DEFAULT 10); This always reproduces if you put NULL as default in the QGIS Project Widget.

@elpaso elpaso self-assigned this Feb 21, 2023
@elpaso
Copy link
Contributor

elpaso commented Feb 21, 2023

Ok, I misunderstood the problem which is that NULL cannot be set as a user defined default value because it falls back to the provider-defined default value (if any).

I understand this might be seen as a bug but it was actually by design, have a look at the note here:

https://github.com/qgis/QGIS/blob/master/src/core/vector/qgsvectorlayerutils.cpp#L570

we fall exactly in that case.

@nyalldawson do you remember the rationale behind that choice?

@nyalldawson
Copy link
Collaborator

you remember the rationale behind that choice?

Because if we don't there's NO way to get serial and other backend sequences populated when eg splitting features.

Perhaps this could be solved if there was a new option "unset value on update" or similar, but that would require some complex logic to handle updates of existing projects...

@elpaso
Copy link
Contributor

elpaso commented Feb 22, 2023

@nyalldawson I understand the motivation behind using if instead of else if but I think the problem here is that a user-defined expression-based default value of NULL is treated differently than any other not-null values.

Shouldn't we have a different code path to distinguish when the value (QVariant) isNull() because it wasn't set by any of the previous if cases and when the value isNull() because it was explicitly set to NULL by the user-defined expression-based default?

@nyalldawson
Copy link
Collaborator

@elpaso

I'd have to dig through the git history, but my recollection is that it HAD to be this way so that users could use NULL expressions as a way of saying "reset the value to unset". Ie there were users who expressly required that a null default expression meant unset vs set to null....

@elpaso
Copy link
Contributor

elpaso commented Feb 22, 2023

@elpaso

I'd have to dig through the git history, but my recollection is that it HAD to be this way so that users could use NULL expressions as a way of saying "reset the value to unset". Ie there were users who expressly required that a null default expression meant unset vs set to null....

Yep, that's what the comment in the code say.

Maybe we should close as a won't fix and try to explain this particular behavior with a tooltip over the expression line edit widget.

@nyalldawson
Copy link
Collaborator

Maybe we should close as a won't fix and try to explain this particular behavior with a tooltip over the expression line edit widget.

I'd lean toward a new option honestly -- I've actually hit this same requirement in the past (and also the requirement for the current behavior), so I'd love to see it resolved in a way which doesn't break either workflow...

@tudorbarascu
Copy link
Member Author

I didn't know about the special "reset the value to unset" and for me is counter intuitive. Is there a way to currently bypass this?

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! Widgets
Projects
None yet
Development

No branches or pull requests

4 participants