#2755 closed defect (fixed)
4 regression failures for sfcgal
Reported by: | robe | Owned by: | colivier |
---|---|---|---|
Priority: | blocker | Milestone: | PostGIS 2.1.4 |
Component: | sfcgal | Version: | master |
Keywords: | Cc: |
Description
Running sfcgal on my 9.4 beta1 windows 32-bit, I see 4 errors:
regress_sfcgal .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_95_diff) sfcgal/empty .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_96_diff) sfcgal/regress_ogc .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_101_diff) sfcgal/tickets .. failed (diff expected obtained: /projects/postgis/tmp/2.2_pg9.4w32/test_103_diff)
my regress output is also a bit of a mess on screen, but I thin that is probably a different issue.
I've attached the related files.
Attachments (3)
Change History (19)
by , 10 years ago
Attachment: | sfcgal_failures_2.2_pg9.4w32.zip added |
---|
comment:1 by , 10 years ago
comment:2 by , 10 years ago
actually I think the tickets issue might be because of our changes in curve support so maybe the sfcgal_ticket script just needs some updating
comment:3 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:4 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
by , 10 years ago
Attachment: | 2755.patch added |
---|
comment:5 by , 10 years ago
Part of the problem came from r12350, which is reverted in the patch.
The rest is from sfcgal regression tests that have not been run before.
This patch has been tested against sfcgal 1.0, 1.0.4 and master. Some tests are expected to fail with master due to bugs that will be fixed in 1.0.5.
follow-up: 7 comment:6 by , 10 years ago
You can't just toss r12350, it's necessary to support extension upgrade, because during upgrade two libpostgis.so libraries are simultaneously loaded and they will collide on the GUC variable and upgrade will halt unless you check and avoid double-defining it.
comment:7 by , 10 years ago
Replying to pramsey:
You can't just toss r12350, it's necessary to support extension upgrade, because during upgrade two libpostgis.so libraries are simultaneously loaded and they will collide on the GUC variable and upgrade will halt unless you check and avoid double-defining it.
I'm working on it, but as far as I can tell, the proposed solution does not seem to work.
When changing postgis.backend, maybe because the custom variable is already defined as a place holder, r13250 causes the assign hook definition to be bypassed. As a result geos function are called instead of sfcgal ones (some tests failures, like ST_Distance(geom, empty) show that).
I asked on pg-hackers (http://postgresql.1045698.n5.nabble.com/modify-custom-variables-td5808161.html) and I was advised to avoid loading the old postgis.so before upgrade.
I tend to agree: if you skip the definition of the custom variable, the assign hook will still point to the old version of the library, and we will end-up calling sfcgal backend from the previous version of postgis*.so
comment:8 by , 10 years ago
comment:9 by , 10 years ago
vmo: I still get regression failures in tickets.sql after applying your patch:
--- sfcgal/tickets_expected 2014-06-24 18:03:47.473976272 +0200 +++ /tmp/pgis_reg/test_103_out 2014-06-24 18:04:32.198439334 +0200 @@ -95,7 +95,7 @@ #835.11|MULTILINESTRING EMPTY #835.12|MULTIPOLYGON EMPTY #650|MULTIPOINT(0 0,1 1,2 2) -#667|SRID=4326;CURVEPOLYGON(CIRCULARSTRING(30 40,-49.2314112161292 32.1963871193548,30 40)) +#667|SRID=4326;CURVEPOLYGON(CIRCULARSTRING(30 40,-50 39.9999999999999,30 40)) #677|1121395 #680|01d107000000000000000024c000000000000049400000000000000040 #681a| @@ -201,7 +201,7 @@ #1398a|POINT(-119.093153 45.632669) #1398b|POINT(-160.137654 77.091608) #1543|MULTILINESTRING((0 0,10 0,10 10,0 0),(0 0))|POLYGON((0 0,10 10,10 0,0 0)) -#1578|f|f +#1578|t|t #1580.1|Point[BS] ERROR: transform: couldn't project point (180 90 0): tolerance condition error (-20) #1580.3|Point[BS] @@ -225,7 +225,7 @@ #1791|4.7 ERROR: ST_Segmentize: invalid max_distance 0 (must be >= 0) ERROR: invalid GML representation -#1957|inf +#1957|not tested, error otherwise because both case staetements are evaluated #1978|3.1416 #1996|{"type":"Point","coordinates":[]} #2001|POLYGON((0 0,0 1,1 1,0 0))
This is with SFCGAL from master branch (1.0.5) and libcgal 4.2-4ubuntu1 (packaged). This is on a 64bit machine.
comment:10 by , 10 years ago
That's expected with sfcgal master. It was meant to force fix befrore 1.0.5 is released. I made some fixes in sfcgal, I'll fix the regress test tomorrow.
by , 10 years ago
Attachment: | 2755_1.patch added |
---|
follow-up: 13 comment:12 by , 10 years ago
Committed as r12654 — should it be backported to 2.1 and 2.0 branches ?
comment:13 by , 10 years ago
comment:14 by , 10 years ago
r12655 is the backport into 2.1 branch, will leave the 2.0 backport to yourself Vincent, ok ?
comment:15 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Actually, SFCGAL support was added in 2.1.0 so all done here.
comment:16 by , 10 years ago
Milestone: | PostGIS 2.2.0 → PostGIS 2.1.4 |
---|
At a quick glance (regress_sfcgal): I honestly can't see a difference so maybe its white space.
other something weird with I guess how empty is compared. Might only be a windows issue:
— regress_ogc_expected - whacked — there is such a thing as a -0 ? I think my result should be right
sfcgal_tickets — don't even know what to make of this:
Anyway if someone else can conrim that these are true errors or bugs in our regress tests itself, that would be appreciated.
I compiled with
I built SFCGAL using CGAL 4.2, Boost 1.53.0, mpfr 3.1.2, gmp 5.1.2