Add optional value parameter(s) to the Force3D*, Force4D functions

[kthy]

Fixes #3057.

Includes documentation update, CUnit tests and regression tests.

[robe2]

Only issue I have is the mix of ordering of cunit tests with this commit.

[kthy]

Only issue I have is the mix of ordering of cunit tests with this commit.

Yeah, I thought of that as well after submitting the PR. Can you cherry-pick the other commits, or would you prefer I recreate in a new branch and submit a new PR without the test suite ordering?

(I'm thinking about extending the CUnit tests now I've found the codecov reports - it would be a better fit in a PR for that. Unless you don't like having the test suites ordered by alpha for some reason. 😉)

[kthy]

Looking through the diffs just now, I notice that it looks like my PR reverts Regina's commit 478f302 in master (postgis/postgis.sql.in lines 2853-2856), even though git informs me my branch is in sync. I'm too tired right now to figure out that part.

[Algunenano]

First of all, thanks for the PR. Here are my notes about it:

• I'm not sure about the defaultZ and defaultM along the whole PR. I think using mvalue or zvalue is clearer, but I don't have a strong position.
• Not too exited about mixing a new feature with the test reordering.
• I think we should try to have the default in the proper place, which is the SQL declaration using a default value, not in multiple places. I think that dropping the internal C functions without values (lwgeom_force_3dz and so on) is the way to go to ensure that.
• Continuing with the previous topic, we could leave a single function per coordinate, i.e. ST_Force3DZ(geom geometry, zvalue float8 default 0.0) and drop the old ones.
• f52cfc3 is reverting a commit while introducing changes.

[kthy]

I think that dropping the internal C functions without values (lwgeom_force_3dz and so on) is the way to go to ensure that.

I avoided that to ensure I didn't need to change other internal calls to those functions, but searching through the source it seems the only other use is in _lwt_AddPoint inside lwgeom_topo.c. That could easily be replaced.

I'll wait for more comments before updating the PR.

[robe2]

I think @Algunenano brought up all the points I care to mention. Only additional thing is if we drop a signature, make sure to put in a error stub for the old one in postgis_legacy.c

[Algunenano]

Only additional thing is if we drop a signature, make sure to put in a error stub for the old one in postgis_legacy.c

I think that in this case we are only dropping internal C functions, but keeping the API ones (since adding a new parameter there doesn't require any changes), just being safe and controlling that we are getting as many input parameters as we need (3).

[robe2]

• I think we should try to have the default in the proper place, which is the SQL declaration using a default value, not in multiple places. I think that dropping the internal C functions without values (lwgeom_force_3dz and so on) is the way to go to ensure that.
• Continuing with the previous topic, we could leave a single function per coordinate, i.e. ST_Force3DZ(geom geometry, zvalue float8 default 0.0) and drop the old ones.

@Algunenano I assume this would necessitate adding a some code to postgis/postgis_before_upgrade.sql to remove the old SQL signature that doesn't take a default?

[kthy]

Urgh, now what did I do? I'm no PR master.

I'll close this mess and submit a new PR shortly, including the requested changes.