ST_AsTWKB doesn't always remove duplicate points

[nicklas]

In this tweet gabrielroldan says that ST_AsTWKB doesn't remove repeated points ​https://twitter.com/gabrielroldan/status/1152379542801174528

and he is right for some cases. If the writer haven't yet collected enough points for what is needed for that geometry type it doesn't remove the point even if there is many more points after.

The function actually needs one extra before it starts removing.

Example

select st_astwkb('linestring(1 1, 2 2, 2 2, 3 1)'::geometry);

The fix is very small, but might be a braking change for someone.

The proposed svn diff:

svn diff
Index: liblwgeom/lwout_twkb.c
===================================================================
--- liblwgeom/lwout_twkb.c	(revision 17618)
+++ liblwgeom/lwout_twkb.c	(working copy)
@@ -112,7 +112,8 @@
 	int64_t nextdelta[MAX_N_DIMS];
 	int npoints = 0;
 	size_t npoints_offset = 0;
-
+	int max_points_left = pa->npoints;
+	
 	LWDEBUGF(2, "Entered %s", __func__);
 
 	/* Dispense with the empty case right away */
@@ -173,8 +174,11 @@
 		/* Skipping the first point is not allowed */
 		/* If the sum(abs()) of all the deltas was zero, */
 		/* then this was a duplicate point, so we can ignore it */
-		if ( i > minpoints && diff == 0 )
+		if ( diff == 0 &&  max_points_left > minpoints )
+		{
+			max_points_left--;
 			continue;
+		}
 
 		/* We really added a point, so... */
 		npoints++;
Index: liblwgeom/measures3d.c
===================================================================
--- liblwgeom/measures3d.c	(revision 17618)
+++ liblwgeom/measures3d.c	(working copy)
@@ -1508,7 +1508,6 @@
 			pl->pv.z += vp.z / vl;
 		}
 	}
-
 	return (!FP_IS_ZERO(pl->pv.x) || !FP_IS_ZERO(pl->pv.y) || !FP_IS_ZERO(pl->pv.z));
 }

Any thoughts before I commit?

[Algunenano]

Any thoughts before I commit?

 +	int max_points_left = pa->npoints;

This should use the same type as pa→npoints (uint32_t).

Index: liblwgeom/measures3d.c
===================================================================
--- liblwgeom/measures3d.c	(revision 17618)
+++ liblwgeom/measures3d.c	(working copy)
@@ -1508,7 +1508,6 @@
 			pl->pv.z += vp.z / vl;
 		}
 	}
-
 	return (!FP_IS_ZERO(pl->pv.x) || !FP_IS_ZERO(pl->pv.y) || !FP_IS_ZERO(pl->pv.z));
 }

This part of the patch is unrelated.

Can you also add tests around this, please?

[nicklas]

In 17654:

Remove duplicate points with ST_AsTWKB also when apearing before min npoints Closes #4461

[nicklas]

Thanks @Algunenano. A 4 row patch and I miss the correct type

[pramsey]

In 17656:

ST_AsTWKB doesn't always remove duplicate points
References #4461

[pramsey]

In 17657:

ST_AsTWKB doesn't always remove duplicate points
References #4461

[murdos]

From my understanding of the patch, the duplicated point is removed for any linestring. However the TWKB specification only talks about this optimization for linearring of polygons.

EDIT: I was wrong, I misunderstood the optimization done in postgis (removing duplicated consecutive points). I thought the optimization was to remove last point of a linear ring.