Bug report #15226

Primary key issue when copying a table by Drag&Drop

Added by Reinhard Reiterer about 3 years ago. Updated almost 3 years ago.

Status:Closed
Priority:Severe/Regression
Assignee:Sandro Santilli
Category:DB Manager
Affected QGIS version:2.16.3 Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:fixed/implemented
Crashes QGIS or corrupts data:No Copied to github as #:23163

Description

When copying a table (PostGIS) in the 'DB Manager' by D&D, the name of the primary key is placed in quotes (e.g. "gid"). As a result, there is a "gid" and a gid column in the new table.

15226.png (53.3 KB) Reinhard Reiterer, 2016-07-07 08:17 AM

15226_screencast.wmv (920 KB) Reinhard Reiterer, 2016-10-12 09:18 AM

15226_screencast_2.mp4 (700 KB) Reinhard Reiterer, 2016-10-12 12:14 PM

qgis_test.backup (10 KB) Reinhard Reiterer, 2016-10-12 12:14 PM

Associated revisions

Revision 5abdfcb8
Added by Sandro Santilli almost 3 years ago

Fix PostgreSQL import of layers with multi-column or quoted-column keys

Fixes #15226 (drag & drop of postgresql views)
Includes test

Revision 1ce09b84
Added by Sandro Santilli almost 3 years ago

Fix PostgreSQL import of layers with multi-column or quoted-column keys

Fixes #15226 (drag & drop of postgresql views)
Includes test

Revision c8b1b18c
Added by Sandro Santilli almost 3 years ago

Fix PostgreSQL import of layers with multi-column or quoted-column keys

Fixes #15226 (drag & drop of postgresql views)
Includes test

Revision ada9348e
Added by Sandro Santilli almost 3 years ago

Fix PostgreSQL import of layers with multi-column or quoted-column keys

Fixes #15226 (drag & drop of postgresql views)
Includes test

History

#1 Updated by Reinhard Reiterer about 3 years ago

#2 Updated by Reinhard Reiterer about 3 years ago

The background shows the input table (see 15226.png).

More D&D issues:

#6798 (when copying a table by d&d from one schema to another the serial property is lost in the pk)
#14712 (DB-Manager DEFAULT value d&d issue)

#3 Updated by Sandro Santilli almost 3 years ago

Reinhard can you help me reproduce the issue ? Where should I drag the table from ? Where should I drag it to ? Which versions did you test ? Can you provide SQL to initialize such example test table ?

#4 Updated by Sandro Santilli almost 3 years ago

  • Status changed from Open to Feedback
  • Assignee set to Sandro Santilli

I've tried dragging the qgis_test.base_table_good table (part of testsuite in qgis_test database) into a new schema and it worked fine with 2.16 branch (2.16.3), 2.14 branch (2.14.7) and master_2 branch (2.17).

#5 Updated by Reinhard Reiterer almost 3 years ago

Hi Sandro, I've uploaded a screencast. Unfortunately, I've no idea how to reproduce this issue.

#6 Updated by Sandro Santilli almost 3 years ago

Interesting, can you inspect the two tables (the one which works and the one which doesn't) with the "psql" command-line database tool ? Can it be the "gid" field is really encoded differently ? Or, can you reproduce after dumping and reloading that table ? Can you spot any other difference between the two tables ? Does it always happen on that table, no matter the order in which you copy them ?

#7 Updated by Reinhard Reiterer almost 3 years ago

Here comes a sample database. Please let me know if this is helpful for locating the issue.

#8 Updated by Sandro Santilli almost 3 years ago

  • Affected QGIS version changed from master to 2.16.3
  • Target version set to Version 2.18
  • Status changed from Feedback to In Progress

Yes, I can reproduce with the table you provided. And not with other tables. I'm on it.
2.14.7 is not affected, 2.16.3 and 2.17 are.

#9 Updated by Sandro Santilli almost 3 years ago

Views are affected, while real table aren't (or so it looks).
The bug was introduced by the commit fixing #13710 (daa6510970e9afbc4d41d28e0c94b4f238eb372d)

\\cc Jurgen

#10 Updated by Sandro Santilli almost 3 years ago

Jurgen, once again this issue is about properly defining semantic for some fields.
In this case is the "keyColumn()" return from QgsDatasourceUri.
Is that supposed to contain quoted or unquoted names ?

#11 Updated by Jürgen Fischer almost 3 years ago

Sandro Santilli wrote:

Jurgen, once again this issue is about properly defining semantic for some fields.
In this case is the "keyColumn()" return from QgsDatasourceUri.
Is that supposed to contain quoted or unquoted names ?

the postgres provider handles quoted and unquoted content from keyColumn() - dbmanager should do too.

#12 Updated by Sandro Santilli almost 3 years ago

On further research, I found that we get quoted identifiers in the url when the primary key type is a FidMap. This happens for multi-column keys or for non-integer keys. In both these cases, even with QGIS 2.14, we get quoted attributes in the QgsDatasourceUri keyColumn() return.

In all these cases, the DBManager drag & drop operation results in a target table having quotes in the key column names.
Even in QGIS 2.14.

I'll see how DBManager should deal with quotes, but given the above I guess the only sane way to deal with quotes would be knowing if the quotes were added or were part of the actual field name, so somehow the "FidMap" nature should be exposed.
or we should always quote the columns ?

BTW, I'm not sure it's DBManager or the provider's "CreateEmptyLayer" being responsible of parsing the URL.
Will keep looking.

#13 Updated by Sandro Santilli almost 3 years ago

I confirm it should be createEmptyTable, within the Postgres provider, to interpret the URI.
Hopefully the semantic of that "key" parameter of the URI will only be needed from within the provider

#14 Updated by Sandro Santilli almost 3 years ago

So with PR https://github.com/qgis/QGIS/pull/3599 I've made PostgreSQL layer importer (createEmptyLayer) support quoted identifiers in Datasource URI -- for backward compatibility with projects created #13710 was fixed (that ticket lacks a target version) -- and I've reverted the commit which did add quoted identifiers in Datasource URI as they do seem redundant and inconsistent (I did test that #13710 isn't back).

In its current state, the code in the PR not only fixes this bug but also adds support for Drag&Dropp'ing tables with multi-column keys (including those with mixed-cased-colum names)

#15 Updated by Sandro Santilli almost 3 years ago

  • % Done changed from 0 to 90

#16 Updated by Reinhard Reiterer almost 3 years ago

Sandro, it's great to see this bug fixed.

Sandro Santilli wrote:

Views are affected, while real table aren't (or so it looks).

As seen in screencast '15226_screencast.wmv' copying tables by D&D may also fail. Is there anything else to do?

#17 Updated by Sandro Santilli almost 3 years ago

  • Status changed from In Progress to Closed

#18 Updated by Sandro Santilli almost 3 years ago

To the best of my knowledge copying tables by D&D also succeeds now. If you can test, the working code is in master_2 branch, since dd85796bae24842da078ad2a3e90f451839db993 - reopen if you still see a problem with Drag&Drop and primary key.

#19 Updated by Sandro Santilli almost 3 years ago

  • Resolution set to fixed/implemented
  • % Done changed from 90 to 100

Also available in: Atom PDF