solved issue #7550

[vmora]

No description provided.

[jef-n]

Sorry, my comments in the ticket apparently were misleading. I forgot about 2a002c5.

Some more:

• manual setting of the primary key should be possible (like in the postgres provider)
• There is an interface to get the primary key columns: QgsVectorDataProvider::pkAttributeIndexes()
• It's not implemented in the spatialite provider yet.
• But there already is a member mPrimaryKey in the provider - so no need for QgsSpatiaLiteProvider::getPrimaryKey(). But that still assumes that there can only be one column making up the primary key - which is not mandatory. Not sure if we can safely assume there always is a ROWID (in that case using that instead of the "real" primary might be preferable - easier to implement than multi attribute keys).
• QgsVectorLayer::splitGeometry() already used to use QgsVectorLayer::pendingPkAttributes(), but that apparently got lost when it was refactored to QgsVectorLayerEditUtils.
• You should run scripts/prepare-commit.sh before commiting (automatic indentation)
• Qt has Q_ASSERT.

[vmora]

On 09/07/2013 23:53, Jürgen Fischer wrote:

Sorry, my comments in the ticket apparently were misleading. I forgot
about commit:2a002c55.

Some more:

• manual setting of the primary key should be possible (like in the
  postgres provider)

I tried that before attempting a fix (opening the attribute table and
editing the values after the split) but the result was strange (one
piece of the cut was reset to the full polygon extent on save).

• There is an interface to get the primary key columns:
  QgsVectorDataProvider::pkAttributeIndexes()
• It's not implemented in the spatialite provider yet.

Do you think this function should be implemented ? Not clear for me
regarding your other comments

• But there already is a member mPrimaryKey in the provider - so no
  need for QgsSpatiaLiteProvider::getPrimaryKey().

My bad, I did not look for table_info in the SL provider, I will remove
the getPrimaryKey() altogether then. Since it is not part of the public
provider interface.

• But that still assumes that there can only be one column making up
  the primary key - which is not mandatory. Not sure if we can
  safely assume there always is a ROWID (in that case using that
  instead of the "real" primary might be preferable - easier to
  implement that multi attribute keys).

I don't get how the ROWID can save us here. And I don't see extreme
difficulty in handling multiple column primary keys if needed.

• QgsVectorLayer::splitGeometry() already used to use
  QgsVectorLayer::pendingPkAttributes(), but that apparently got
  lost when it was refactored to QgsVectorLayerEditUtils.

Do you think it's better to go back to this design, or just check, in
addFeatures that added features have a unique pk (that can be multiple)
and if not set the pk to NULL?

• You should run scripts/prepare-commit.sh before commiting
  (automatic indentation)
• Qt has Q_ASSERT.

Sorry fot that, newby here, must have missed that in the dev doc. I
guess those two mistakes could easily be avoided if two tests:

• one that fails if assert is found,
• one that fails if indentation is inconsistent
  were added to the test suite.

[vmora]

I'm preparing a better fix for the issue, and considering jef's remarks I close the PR to avoid waisting time on it.