Pasting features into PostGIS table doesn't copy primary key

jesse-a-reilly commented 2 years ago

What is the bug or the crash?

When pasting features into a PostGIS layer, the primary key field is left blank. The Fix Pasted Features window appears, asking to fill in the primary key field.

In this instance the "service_no" field is included in both the source and target layers.

Removing the primary key constraint on the target table lets the attributes be pasted correctly. So for the time being, I can remove the constraint, paste the features, and re-add the constraint, though is isn't ideal!

Postgresql version: PostgreSQL 10.19 (Ubuntu 10.19-1.pgdg18.04+1) on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0, 64-bit

PostGIS version: 2.4 USE_GEOS=1 USE_PROJ=1 USE_STATS=1

Steps to reproduce the issue

Select features from a vector layer
Copy the features
Select a postGIS layer in the TOC
Paste Features

Versions

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd">

Supported QGIS version

[X] I'm running a supported QGIS version according to the roadmap.

New profile

[X] I tried with a new QGIS profile

Additional context

Running on Mac with Apple M1 CPU

gioman commented 2 years ago

Select features from a vector layer

Copy the features

Select a postGIS layer in the TOC

Paste Features

@jesse-a-reilly tried on 3.22.1 and cannot replicate.

Name	Type	Column(s)
mylayer_pkey	Primary key	gid

github-actions[bot] commented 2 years ago

The QGIS project highly values your report and would love to see it addressed. However, this issue has been left in feedback mode for the last 14 days and is being automatically marked as "stale". If you would like to continue with this issue, please provide any missing information or answer any open questions. If you could resolve the issue yourself meanwhile, please leave a note for future readers with the same problem and close the issue. In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this issue. If there is no further activity on this issue, it will be closed in a week.

github-actions[bot] commented 2 years ago

While we hate to see this happen, this issue has been automatically closed because it has not had any activity in the last 42 days despite being marked as feedback. If this issue should be reconsidered, please follow the guidelines in the previous comment and reopen this issue. Or, if you have any further questions, there are also further support channels that can help you.

Swederman commented 2 years ago

I have the exact same issue with 3.16.8 :

3 16

However it works fine using 3.10.11 :

3 10

Both times using the same data source (from PostgreSQL 13.4.2 and Postgis 3.14.1)

gioman commented 2 years ago

I have the exact same issue with 3.16.8 :

@Swederman 3.16 is EOL, you must test with 3.22.4 or 3.24.0

jesse-a-reilly commented 2 years ago

I'm still experiencing this in QGS-LTR 3.22.7-Białowieża Are there any steps I can take, or any information that I can look for which would help troubleshoot this?

andreasneumann commented 2 years ago

@jesse-a-reilly and @Swederman - is this about preserving existing keys (potentially leading to uniqueness violations) or about generating a new key?

Are you aware of the setting "Evaluate default values on provider side" in menu "Project" --> "Data Sources" ? It basically means that you will get a new primary directly from the provider (postgis in your case), even before saving.

If you enable both settings "Automatically create transaction groups where possible" and "Evaluate default values on provider side" - does this improve the situation?

Swederman commented 2 years ago

Hi @andreasneumann, thanks for your reply

@jesse-a-reilly and @Swederman - is this about preserving existing keys (potentially leading to uniqueness violations) or about generating a new key?

For me it's about generating a new key when duplicating entities

If you enable both settings "Automatically create transaction groups where possible" and "Evaluate default values on provider side" - does this improve the situation?

I've tried it with both 3.16.8 and 3.22.4, it doesn't change the situation, I still have the pop-up windows requiring a manual id key input

jesse-a-reilly commented 2 years ago

@andreasneumann For me I want to preserve existing keys.

I suspect this might only be an issue on tables that don't have an auto-incremented primary key field - I've been able to copy features to tables where the primary key field is a SERIAL type.

I've tried turning on Evaluate default values on provider side without any luck. I'm unable to turn on Automatically create transaction groups where possible - the option is greyed out.

github-actions[bot] commented 2 years ago

The QGIS project highly values your report and would love to see it addressed. However, this issue has been left in feedback mode for the last 14 days and is being automatically marked as "stale". If you would like to continue with this issue, please provide any missing information or answer any open questions. If you could resolve the issue yourself meanwhile, please leave a note for future readers with the same problem and close the issue. In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this issue. If there is no further activity on this issue, it will be closed in a week.

github-actions[bot] commented 2 years ago

While we hate to see this happen, this issue has been automatically closed because it has not had any activity in the last 42 days despite being marked as feedback. If this issue should be reconsidered, please follow the guidelines in the previous comment and reopen this issue. Or, if you have any further questions, there are also further support channels that can help you.

karinelsg commented 2 years ago

Hello,

We encounter the same issue with QGIS LTR 3.22.7. We try to copy data from a shapefile to a postgis table with an integer as primary key but the primary key is set to NULL in QGIS.

We get this result in QGIS :

We need to keep the initial integer (id column) as primary key, we are not willing to create an auto-incremented primary key in postgis for this specific table.

We use :

QGIS LTR 3.22.7
PostgreSQL 9.6.9 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18), 64-bit
PostGIS 2.4.4

I tried in QGIS :

to make the id column non-editable in the layer properties
to change the project properties : check the box "Evaluate default values on provider side"

Both didn't fix the issue.

I tried to copy paste the same data in the same postgis table with QGIS LTR 3.16.4 and it worked fine.

karinelsg commented 2 years ago

Hello, would there be a way to re-Open this issue ? It would be really helpful to fix it for the next LTR version. Thanks !

karinelsg commented 2 years ago

Hello, I repost our problem with a short sql script to reproduce more easily.

Context

We often work with two layers in QGIS : a work vector layer and a production table in PostGIS.

On the work layer we have an id (often an auto-incremented integer)
On the production table we have an id as primary key (ofter an integer)

We copy-paste data from work layer to production table in QGIS. Ids from the two layers must be the same.

What is the bug or the crash?

When pasting features into a PostGIS layer, the primary key field is left blank. The Fix Pasted Features window appears, asking to fill in the primary key field.

Steps to reproduce the issue

In QGIS,

select features from a vector or Postgis layer
copy features
select Postgis layer with defined primary key
paste features

Versions

QGIS LTR 3.22.7
PostgreSQL 9.6.9 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18), 64-bit
PostGIS 2.4.4

Attached

A sql dump to be restored with psql : In a schema named test_velo you will find

-1- a work table named segment_cyclable_w containing 3 entites with 2 attributes (id_local pk, vvv_statut) and geometry -2- an empty production table named segment_cyclable (with primary key) : copy paste should fail on that table -3- an empty table named segment_cyclable_without_pk : copy paste should work on that table, without primary key

Copy fails with primary key in postgis :

Copy succeeds without primary key in postgis :

Thanks in advance,

221026_dump_schema_test_velo_sql.txt

karinelsg commented 1 year ago

I still encounter the same issue when testing with QGIS 3.28.0

andreasneumann commented 1 year ago

@karinelsg - it s not obvious if this is a bug or not. As you see above in the discussion - some (a lot?) of users don't expect the primary key to be preserved but regenerated - because you often paste into other tables that have totally different primary key sequences and just pasting primary keys from other tables may result in primary key clashes (duplicates).

If you want to preserve primary keys when pasting, it has to be an "opt in" option, because otherwise you would break workflows of many other users.

karinelsg commented 1 year ago

@andreasneumann I consider it as a bug as previous QGIS versions offered the possibility to preserve primary keys. We based some of our workflows on this.

We have a work table with auto-incremented integer primary keys and a production table with integer primary keys. Data pasted into that production table are only coming from that work table, so duplicates are avoided thanks to the primary key sequence on the work table.

Users willing to regenerate primary keys can simply create an auto-incremented primary key on the destination table (we work this way on many of our tables as well, when we do not wish to keep the inital sequence).

It was very useful to be able to cover the two scenarios.

andreasneumann commented 1 year ago

@nyalldawson - you recently worked on improving the autoincrement / pkey situation in recent QGIS and probably know the situation quite well. What do you think about the discussions here? Do you see a good solution to offer both ways: either preserve or regenerate the primary key?

sigcd31 commented 1 year ago

I agree with karinelsg This is a regression with 3.22. QGis 3.16 was perfect for dealing PosgreSQL copy/paste with "explicit" primary key.

Primary keys are often explicit and not simply an auto incremented number !

We have a lot of tables that are managed with copy/paste from a working table with explicit primary key. This bug is very very annoying for us and we hope the next LTR QGis will put things back in place

Many thanks !

Fabrice

nyalldawson commented 1 year ago

@andreasneumann

Do you see a good solution to offer both ways: either preserve or regenerate the primary key?

Not really, beside exposing it as a user opt-in setting.

We could potentially detect whether a primary key from one table is being added again to the same table vs a different table and allow the copy only if it's going to a different table. But that won't work everywhere either, because there's no guarantee that the key from the source table doesn't already exist in the destination.

So there's nothing "smart" we can do here and would have to throw it back to a user to explicitly allow the copy....

andreasneumann commented 1 year ago

@jesse-a-reilly - @karinelsg or @sigcd31 - since you are interested in this - do you have a QGIS developer or QGIS support company at hand that can help you get this implemented as an "opt in" when pasting features?

sigcd31 commented 1 year ago

@andreasneumann It's frustrating in 2 ways because :

It's a QGis regression, it used to work fine till 3.16 (with explicit and autoincremented primary key)
From a database point of view, it's an aberration to consider that primary keys have to be auto incremented, in the real world they are often "explicits". Fabrice

andreasneumann commented 1 year ago

@sigcd31 - I understand it is frustrating for you - but it was also very frustrating for other users that it was automatically assumed that primary keys are pasted/kept by default. It generated a lot of error messages due to primary key conflicts.

Users very often copy paste from totally different data sets (e.g. from a Shapefile or Geopackage) to a Postgis table - and taking over primary keys would very often result in error messages. Or copy from one Postgis table to another one that has a different primary key scheme, and it would result in pkey clashes.

Because of that I think users should very consciously use the option to take over primary keys from the original table. I am not saying that there aren't use cases for it - obviously there are - and I also sometimes carry over pkeys from one table to the other in my SQL scripts.

So we need to find a way to get back the functionality that you want - as an opt-in.

I am just trying to moderate here. I am not the dev who introduced what you call a "regression". QGIS nowhere claims to preserve primary keys. It's not that it is written in the manual that primary keys are preserved when you copy paste between layers.

sigcd31 commented 1 year ago

@andreasneumann thanks for your explanations. I did a lot of tests today and I must apologize, even in 3.16 Qgis was not preserving pkeys as it should be... We work with Posgis

With 3.16 : if I copy a record with a pkey value = "31DD" in a table that doesn't contains this value, it is OK But if I copy a record with a pkey value = "31DD" in a table that already contains this value, QGis alter the pkey as "31DD_1" instead of returning an error message from PostgreSQL. Very very dangerous !!! no message and the pkey is changed !!

QGis 3.22 don't manage existing pkeys at all (when copy/paste between 2 tables), but it is better and safer than with 3.16 QGis 3.22 is better than 3.16 and is not a regression at all.

From my opinion, QGis should let PostgreSQL deal with primary keys and QGis should not modify a primary key

We will use an other way to copy/paste from a working table to a production table (FME or SQL do the job as it should be done) Cheers Fabrice

andreasneumann commented 1 year ago

But if I copy a record with a pkey value = "31DD" in a table that already contains this value, QGis alter the pkey as "31DD_1" instead of returning an error message from PostgreSQL.

I agree - that is not a good behavior to silently rewrite pkeys.

Swederman commented 1 year ago

The auto-incrementation from a PostreSQLdefined primary key sequence worked perfectly fine in 3.10 and doesn't since.

And from what @sigcd31 and @karinelsg are saying it doesn't work either for keeping existing primary key, so it would be interesting to go back and see what changed from 3.10 to 3.12/3.16

For now the only solution I have is to keep using 3.10 whenever I need to copy/paste features.

andreasneumann commented 1 year ago

The auto-incrementation from a PostreSQLdefined primary key sequence worked perfectly fine in 3.10 and doesn't since.

@Swederman - are you sure? Do you have proof with an example project and data? What version exactly?

I justed tested this in version 3.28.x - copying over 3 parcel polygon objects into another layer with different pkey sequences and it worked just fine. It generated the new keys from the sequence of the destination layer just fine.

As far as I understand the issue, this is now only about preserving the existing primary key. And if this is still required we need to find a developer that improves the situation such, that there is an option (either on the layer or for any past operation) to opt-in for the preserving of the primary keys coming from the source layer.

jesse-a-reilly commented 1 year ago

@jesse-a-reilly - @karinelsg or @sigcd31 - since you are interested in this - do you have a QGIS developer or QGIS support company at hand that can help you get this implemented as an "opt in" when pasting features?

Unfortunately I'm the closest our company has to a developer, and this one is quite above my skillset to take on. But I appreciate this issue being picked up again - I like the idea of preserving primary keys being an opt-in setting, whether it's set QGIS-wide, per project, or per layer.

sigcd31 commented 1 year ago

@Swederman +1 Yes it was QGis 3.10 that worked fine with primary keys What has changed since 3.10? And why such a regression?

nyalldawson commented 1 year ago

What has changed since 3.10? And why such a regression?

In this case one person's "regression" was many user's bug fix.

It was an intentional change made to fix a real issue.

karinelsg commented 1 year ago

I tested with QGIS 3.10 > it works the same as 3.16, it pastes the primary key but if the PK is already in the table, it changes it (whether it is integer or text primary key in the destination table).

I never realised it worked this way so I agree that this was not a proper functioning and it had to be fixed.

However, QGIS works the same when there is a UNIQUE constraint in PostGIS ! If the value already exists in the table, it is changed by QGIS... (still happens in QGIS 3.22 and 3.28)

I still think that not being able to copy data with PK or UNIQUE constraint is annoying and should rely on PostGIS and not on QGIS.

For me a proper functioning would be :

paste in QGIS in the destination table the exact same values that are in the source table
when saving the data, get an error message because of duplicate PK or duplicate values on columns with UNIQUE constraint (like in PgAdmin)

EDIT : users willing to regenerate PK can have a sequence (serial) on their destination table in PostGIS... it works fine

I don't have good enough development skills to implement this evolution but I think is is quite dangerous that data are being changed by QGIS. We will think internally about another process to carry out those kind of operations (FME maybe).

Karine

sigcd31 commented 1 year ago

What has changed since 3.10? And why such a regression?

In this case one person's "regression" was many user's bug fix.

It was an intentional change made to fix a real issue.

Primary keys and unicity constraints are made to ensure the database integrity. If you try to insert a record that already exists, the database says no, it is not possible.

QGis is the only software I know that changes the primary key value if it already exists, to avoid de database check... If you insert 3 times a primary key 'thekey' that already exists, QGis will lie to the database and will insert 'thekey1, 'thekey2', 'thekey3'.

With QGis it is not possible to guarantee the integrity of the database.

jeabraham commented 1 year ago

In my opinion assuming the user doesn't want to paste the primary key is a weird assumption. If the field is an auto increment field, I guess it might be a reasonable assumption, but in any other situation I can't see it.

What I've done to get around this is create a view that joins my real table to another table. This other table serves no purpose other than to maintain a one-to-one relationship to auto-increment field to trick QGIS. Then, I create a trigger function on this view, so that when QGIS attempts to insert into this view, the trigger function will insert the correct primary key into the real table, and also create a new entry with the auto increment ID into the trick one-to-one table.

QGIS still complains, it wants me to specify the id field which is an auto increment field. But if I select "paste all, including invalid", and then save the edits, the trigger function does the right thing, inserts the right value into the real primary key field, creates a value in the id field using the auto increment, and we've managed to trick QGIS into allowing us to paste values into fields that are primary keys.

I hope this mechanism helps someone else until the feature request (of being able to paste into a primary key field) is implemented.

rkolka commented 1 month ago

In my opinion assuming the user doesn't want to paste the primary key is a weird assumption.

100%

the feature request (of being able to paste into a primary key field) is implemented

Is there a link to the feature request?

qgis / QGIS