[ogr2ogr] Fix promote to multipart logic and add makevalid option

[NyakudyaA]

Fixes #58392

• Add an option for makevalid, off by default
• Raise an exception if Promote and output geometry type are both selected
• Raise an exception if Append and Overwrite are both selected

[github-actions]

🪟 Windows builds ready!

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

(Built from commit d0f078a)

[agiudiceandrea]

Maybe it could be better to inform the user or raise an exception about the fact that they has selected the two incompatible options GTYPE and PROMOTETOMULTI (also when the algorithm is called by a Python script or it is used in a module).
It looks like the CONVERT_TO_LINEAR and CONVERT_TO_CURVE options are missing for the -nlt parameter, anyway they could be added as additional command line parameter.
Isn't the "Export to PostgreSQL (new connection)" (gdal:importvectorintopostgisdatabasenewconnection) also affected?
What about the "overwriting the existing table and appending to an existing table that should be mutually exclusive" reported in the same issue report?

[NyakudyaA]

Maybe it could be better to inform the user or raise an exception about the fact that they has selected the two incompatible options GTYPE and PROMOTETOMULTI (also when the algorithm is called by a Python script or it is used in a module). It looks like the CONVERT_TO_LINEAR and CONVERT_TO_CURVE options are missing for the -nlt parameter, anyway they could be added as additional command line parameter. Isn't the "Export to PostgreSQL (new connection)" (gdal:importvectorintopostgisdatabasenewconnection) also affected? What about the "overwriting the existing table and appending to an existing table that should be mutually exclusive" reported in the same issue report?

I updated the other script and added the missing import options for GTYPE. Now we get the following

[agiudiceandrea]

Thanks!
According to https://gdal.org/programs/ogr2ogr.html#cmdoption-ogr2ogr-nlt it looks like that "-nlt CONVERT_TO_LINEAR and -nlt PROMOTE_TO_MULTI can be used simultaneously."

[NyakudyaA]

Thanks! According to https://gdal.org/programs/ogr2ogr.html#cmdoption-ogr2ogr-nlt it looks like that "-nlt CONVERT_TO_LINEAR and -nlt PROMOTE_TO_MULTI can be used simultaneously."

fixed:

[agiudiceandrea]

@rouault it looks like the -overwrite and the -append options for ogr2ogr has been declared as mutually exclusive only since GDAL/OGR 3.9.0, rising the Argument '-overwrite' not allowed with '-append' error, while until GDAL/OGR 3.8.5 they could be both present at the same time without rising an error (in such case it seems to me that only the last one in the command was taken in consideration). Is / was it the intended behaviour? Do you think the algorithm should allow -append -overwrite when used with GDAL/OGR < 3.9.0 and only rise an exception for GDAL/OGR >= 3.9.0, or should it always rise an exception regardless of the GDAL/OGR version used?

[rouault]

-overwrite and -append have always been mutually exclusive. It is just that GDAL 3.9 starts making it an error. In previous versions, the last specified argument would win over the previously secified. It makes sense for QGIS to treat them as mutually exlusive for any GDAL version

[NyakudyaA]

@agiudiceandrea, should we also handle the -overwrite and -addfields as mutually exclusive? Should we also add some validation logic for the Additional creation options, I assume if future gdal versions error in -overwrite and -append, then it should be handled or do we let users handle this since this option is assumed to be useful for advanced users

[agiudiceandrea]

should we also handle the -overwrite and -addfields as mutually exclusive?

It look like they are not actually enforced as mutually exclusive in ogr2ogr, but probably they should be.

Should we also add some validation logic for the Additional creation options

AFAIK there is no validation in other algorithms for the additional creation options, but I've not actually checked.

Please have a look at the failing test.

[qgis-bot]

@NyakudyaA
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[NyakudyaA]

Not stale

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[NyakudyaA]

Not stale

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[NyakudyaA]

Not stale

[nyalldawson]

@agiudiceandrea was there more here you wanted to see?

[github-actions]

🪟 Windows builds

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

🪟 Windows Qt6 builds

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

[agiudiceandrea]

Looks good to me.

[qgis-bot]

@NyakudyaA
A documentation ticket has been opened at qgis/QGIS-Documentation#9307
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!