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

Fix pasting unsaved changes as temporary scratch layers #57985

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

uclaros
Copy link
Contributor

@uclaros uclaros commented Jul 4, 2024

Description

Pasting unsaved changes as a temporary scratch layer could fail when source is a db.
Geopackages would have Autogenerate as an fid value, Postgres layers would have nextval(... values etc, which could not be stored in the target int field.

Instead of failing, this PR continues with a null value instead.

Fixes #38913

@uclaros uclaros added Bug Either a bug report, or a bug fix. Let's hope for the latter! backport release-3_38 labels Jul 4, 2024
@github-actions github-actions bot added this to the 3.40.0 milestone Jul 4, 2024
Copy link

github-actions bot commented Jul 4, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 1a0d81d)

@nyalldawson
Copy link
Collaborator

Does this work if you modify QgsField::convertCompatible to add something like this just after the initial null check?

  if ( QgsVariantUtils::isNull( v ) )
  {
    v.convert( d->type );
    return true;
  }

  if ( v.userType() == QMetaType::type( "QgsUnsetAttributeValue" ) )
  {
    return true;
  }

@uclaros
Copy link
Contributor Author

uclaros commented Jul 5, 2024

Autogenerate and nextval(...) are of userType() == QMetaType::Type::QString, so that will not work.

@nyalldawson
Copy link
Collaborator

@uclaros

Autogenerate and nextval(...) are of userType() == QMetaType::Type::QString,

They shouldn't be! Are you checking the field type or value type?

@uclaros
Copy link
Contributor Author

uclaros commented Jul 5, 2024

Are you checking the field type or value type?

Value type is string, field type should be int.

I can see QgsUnsetAttributeValue is only used when splitting and duplicating features. It's not clear to me how this class should be used.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 20, 2024
@qgis qgis deleted a comment from github-actions bot Jul 24, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 24, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 8, 2024
@qgis qgis deleted a comment from github-actions bot Aug 12, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 12, 2024
@uclaros
Copy link
Contributor Author

uclaros commented Aug 12, 2024

@nyalldawson when convertCompatible() fails, it updates the value to the appropriate null QVariant and returns false.
In this case of pasting to the memory provider, we don't really care about the return value: even if the Autogenerate/nextval was of metatype QgsUnsetAttributeValue and it was handled properly as you suggest, it should still update the value to the appropriate null QVariant or it would fail to be stored on the int field.
Hence my suggestion to completely skip the return value check.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 27, 2024
@qgis qgis deleted a comment from github-actions bot Aug 28, 2024
@uclaros uclaros removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 28, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 12, 2024
@nyalldawson nyalldawson added Freeze Exempt Feature Freeze exemption granted and removed backport release-3_38 labels Sep 13, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 14, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 28, 2024
@qgis qgis deleted a comment from github-actions bot Oct 1, 2024
@qgis qgis deleted a comment from github-actions bot Oct 1, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Oct 1, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Oct 16, 2024
@nyalldawson nyalldawson added Frozen Feature freeze - Do not merge! and removed Freeze Exempt Feature Freeze exemption granted labels Oct 17, 2024
@nyalldawson nyalldawson removed stale Uh oh! Seems this work is abandoned, and the PR is about to close. Frozen Feature freeze - Do not merge! labels Oct 17, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 1, 2024
@qgis qgis deleted a comment from github-actions bot Nov 8, 2024
@qgis qgis deleted a comment from github-actions bot Nov 8, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 8, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 23, 2024
@qgis qgis deleted a comment from github-actions bot Nov 26, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 26, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Dec 11, 2024
@qgis qgis deleted a comment from github-actions bot Dec 11, 2024
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Dec 11, 2024
@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Dec 26, 2024
@github-actions github-actions bot closed this Jan 3, 2025
@qgis qgis deleted a comment from github-actions bot Jan 3, 2025
@qgis qgis deleted a comment from github-actions bot Jan 3, 2025
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jan 3, 2025
@uclaros uclaros reopened this Jan 3, 2025
Copy link

github-actions bot commented Jan 3, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 1a0d81d)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 1a0d81d)

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

Successfully merging this pull request may close these issues.

Cannot paste unsaved changes as temporary scratch layer
2 participants