Test for ST_AddEdgeNewFaces fails under machine load

[strk]

If a postgresql backend is under heavy load, the test for ST_AddEdgeNewFaces fails . Maybe a missing ORDER BY somewhere …

[strk]

The failure:

 regress/st_addedgenewfaces. failed (diff expected obtained: /tmp/pgis_reg_30611/test_1_diff)
-----------------------------------------------------------------------------  
--- regress/st_addedgenewfaces_expected 2011-07-04 15:29:33.000000000 +0200
+++ /tmp/pgis_reg_30611/test_1_out      2011-09-21 22:53:00.000000000 +0200
@@ -50,17 +50,17 @@
 T7|E19|33|27|22|10
 T7|E21|6|9|0|22
 T7|E33|-21|-6|22|23
-T8|E9|-34|-32|25|20
-T8|E19|33|27|24|10
-T8|E21|6|34|0|24
-T8|E34|19|9|24|25
+T8|E9|-34|-32|24|20
+T8|E19|33|27|25|10
+T8|E21|6|34|0|25
+T8|E34|19|9|25|24
 T9|E12|-31|22|20|0
 T9|E13|18|-35|16|0
 T9|E35|35|-12|26|0
 T10|E13|18|-35|16|0
 T10|E14|16|36|14|0
 T10|E36|-13|-36|0|27
-T11|E21|6|34|0|24

[strk]

The order by is there, but is based on minx/miny of bounding box, which in this case are the same (ouch). The annoying thing is that any ordering serves predictability but not performance. Anyway, when an added edge splits 2 faces… _which_ face should we add first ? For the AddEdgeModFace function we retain the right face, maybe for symmetry we should _add_ the right face first in the AddEdgeNewFace..

[aperi2007]

Replying to strk:

The order by is there, but is based on minx/miny of bounding box, which in this case

are the same (ouch). The annoying thing is that any ordering serves predictability but not performance.

I'm not sure to fully understand but I guess perhaps an order based on idface number sometime like "idface1 < idface2"

could be more fast, But I dont know if is right for this case.

Anyway, when an added edge splits 2 faces… _which_ face should we add first ? For the AddEdgeModFace function we retain the right face, maybe for symmetry we should _add_ the right face first in the AddEdgeNewFace..

+1

[strk]

Using the ST_GetRingEdges it would be more elegant and probably also faster (reduce some significant geos calls). Only, it would then work _only_ for properly linked topologies, that is those in which next_left_face/next_right_face are correct.

Mind you: AddEdge/AddFace functions do _not_ set those values correctly, and ValidateTopology doesnt' check for their correctness.

[strk]

r7883 makes face creation order predictable (right face first)

[strk]

Confirmed, is now stable under heavy load.