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

[gpkg] Fix adding field comments after alternative name #8726

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

nyalldawson
Copy link
Collaborator

Fix constraints on gpkg_data_columns are violated when AlterFieldDefn is called in some circumstances, eg when a comment is added after an alternative name has already been set on the field.

@nyalldawson nyalldawson force-pushed the alter_comment_after_alt_name branch 2 times, most recently from 468fdfe to 39bde2a Compare November 16, 2023 03:56
@coveralls
Copy link
Collaborator

coveralls commented Nov 16, 2023

Coverage Status

coverage: 67.861% (+0.003%) from 67.858%
when pulling 8eea753 on nyalldawson:alter_comment_after_alt_name
into b2015f8 on OSGeo:master.

autotest/ogr/ogr_gpkg.py Outdated Show resolved Hide resolved
autotest/ogr/ogr_gpkg.py Outdated Show resolved Hide resolved
@nyalldawson nyalldawson force-pushed the alter_comment_after_alt_name branch from 39bde2a to 8eea753 Compare November 16, 2023 22:44
Fix constraints on gpkg_data_columns are violated when AlterFieldDefn
is called in some circumstances, eg when a comment is added
after an alternative name has already been set on the field.
@nyalldawson
Copy link
Collaborator Author

Thanks @rouault , fixed

@rouault
Copy link
Member

rouault commented Nov 16, 2023

I've seen this is is likely related to qgis/QGIS#55304 . How does the GeoPackage driver reacts when having duplicated aliases: does it error out cleanly and return things in a consistent state?

@nyalldawson
Copy link
Collaborator Author

@rouault

I've seen this is is likely related to qgis/QGIS#55304

Not really related -- you can easily trigger the bug from raw gdal api by setting an alternative name, and then setting a comment in a second step. Eg from qgis if you set alias/comment via the GUI then it'll be two atomic operations like this, and the second will error out with the constraint violation.

How does the GeoPackage driver reacts when having duplicated aliases: does it error out cleanly and return things in a consistent state?

It errors out when trying to set the second field alias, with a message about the failed constraint. It's a relatively technical error message (it's the raw constraint violation message), but it DOES give sufficient information to see what's happening. Nothing is changed, so the GPKG remains valid.

@rouault rouault merged commit 07a27af into OSGeo:master Nov 17, 2023
31 checks passed
@nyalldawson nyalldawson deleted the alter_comment_after_alt_name branch November 17, 2023 00:55
@nyalldawson
Copy link
Collaborator Author

Thanks @rouault !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants