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

a feature copy/paste sets a NULL field to its "default value" in a geopackage #44544

Open
faridcher opened this issue Aug 3, 2021 · 19 comments
Assignees
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers

Comments

@faridcher
Copy link

faridcher commented Aug 3, 2021

What is the bug or the crash?

A feature copy and paste should also carry over all attributes value but the process sets a NULL field value to its default value which is set in its layer Attribute Form.

Steps to reproduce the issue

use the sample labs layer and its apartment field as a basis; its widget type is set to Checkbox and its default value to true (Layer properties-Attribute Form). Use the only feature in the layer with apartment=NULL. Copy and paste this feature to observe that the apartment field value of the new feature is set to true instead of NULL.

Versions

Qgis 3.16.8
@gioman also tested with master

Additional context

sample geopackage data: test.zip
@gioman could not not replicate with a PostGIS layer.

@faridcher faridcher added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Aug 3, 2021
@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

Qgis 3.10.14

@faridcher this version is EOL, you must test on 3.16 or 3.20

A feature copy and paste

@faridcher are you copying/pasting within the same layer or between different ones?

@gioman gioman added the Feedback Waiting on the submitter for answers label Aug 3, 2021
@faridcher

This comment has been minimized.

@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

Sorry I am testing in Debian unstable and cannot compile the dev

@faridcher 3.16 is the stable version, 3.10 is EOL.

@faridcher

This comment has been minimized.

@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

I will verify with a later version if it is too much a hassle.

@faridcher could not, but you could attach a sample layer for examples, so we can test on the same data.

@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

@faridcher works fine here on a PostGIS layer on QGIS master on Ubuntu 20.04.

@faridcher

This comment has been minimized.

@faridcher faridcher changed the title a feature copy/paste sets a NULL field to its "default value" a feature copy/paste sets a NULL field to its "default value" in a geopackage Aug 3, 2021
@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

I upgraded to 3.16 .8 and I hit another bug!

@faridcher another bug, another ticket ;)

@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

check with the apartment field of the layer.

@faridcher yes confirmed.

@gioman gioman added Data Provider Related to specific vector, raster or mesh data providers and removed Feedback Waiting on the submitter for answers labels Aug 3, 2021
@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

A boolean field with NULL values is wrongly shown as false in the 'Attribute Table';

@faridcher what field in your sample?

@faridcher

This comment has been minimized.

@gioman
Copy link
Contributor

gioman commented Aug 3, 2021

@gioman the same apartment field

@faridcher that is an effect of the "boolean" widget, if you set the edit widget to "text edit" you'll see the NULL value.

@faridcher

This comment has been minimized.

@gioman
Copy link
Contributor

gioman commented Aug 4, 2021

a boolean field could also take NULL so I think the widget should render NULLs correctly, don't you think?

@faridcher yes makes sense, anyway file a separate ticket please.

@faridcher
Copy link
Author

faridcher commented Aug 4, 2021

@gioman Could you please hide all of the outdated comments to keep the issue tidy for the prospective bug hunter? I updated the question to reflect the outcome of our discussion. thank you

@troopa81 troopa81 self-assigned this Sep 28, 2021
@troopa81
Copy link
Contributor

@faridcher It's because you have defined a default value in your attribute form appartement configuration. So when you copy the feature, appartement attribute is NULL, so QGIS considers it has to use the defined default value.

You could possibily argue that is not the expected behavior and the new feature should be identical to the copied one, but maybe other users would expect to use the default value on a NULL field. I think I would favor the first one.

It looks like it has been the matter of debate. @m-kuhn @elpaso Do you remember what was the outcome of the discussion about that?

I can propose a fix, if we agree that we don't evaluate default value when user copy-paste feature

@nyalldawson
Copy link
Collaborator

if we agree that we don't evaluate default value when user copy-paste feature

That would be a very bad idea -- default values are used in workflows to do things like set a "feature created" field, which MUST be done when pasting features.

@m-kuhn
Copy link
Member

m-kuhn commented Sep 28, 2021

It looks like it has been the matter of debate. @m-kuhn @elpaso Do you remember what was the outcome of the discussion about that?

The discussion there was different "if qgis default value returns null should we fallback to provider default value".

That would be a very bad idea -- default values are used in workflows to do things like set a "feature created" field, which MUST be done when pasting features.

Does that work? If there is a feature with a created_date already set to a value and it's copy/pasted, the field will not be updated.
Or are you talking about: if there is a target layer B with a field f and a source layer A without this field, default values must be inserted for field f on pasting. That's a requirement I've seen often.

@faridcher
Copy link
Author

faridcher commented Sep 29, 2021

You could possibly argue that is not the expected behavior and the new feature should be identical to the copied one, but maybe other users would expect to use the default value on a NULL field. I think I would favor the first one.

@Troopa I also favor the first one. As the name implies (copy/paste), the target feature in the target layer should be as similar as possible to the source feature even its date_created field. I often copy/paste a feature to only change a single attribute of the pasted feature; with the NULL fields being set to their defaults, I am further obliged to set them to NULL manually. The user always can use the Create Feature tool if they want the default values; they then may utilize Merge Feature Attributes tool to match certain attributes. Or they may force a default value expression by updating a feature assuming the Apply default value on update option is checked in Layer properties-Attributes Form.

Disabling the default values expression altogether might preclude @m-kuhn described scenario (pasting across layers):

if there is a target layer B with a field f and a source layer A without this field, default values must be inserted for field f on pasting. That's a requirement I've seen often.

So the ultimate solution could be: upon paste, evaluate all default values (only when pasting across layers) and then paste all attributes including NULLs from the source feature in the source layer.

@Troopa thank you for looking at 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! Data Provider Related to specific vector, raster or mesh data providers
Projects
None yet
Development

No branches or pull requests

5 participants