Using Unique and Using SRID together breaks CVS HEAD

[bill@…]

It seems that the recent cleanup in mappostgis.c (jerryp's stuff) introduced a bug.

If you use both 'using unique gid' and 'using SRID=1234', then only the last one
will be stripped before it's sent to PostGIS.  Therefore, PostgreSQL blow up
saying it's invalid syntax.  I backed out to version 1.55 of mappostgis.c and
the problem went away.

It is not a case sensitivity thing, and it doesn't matter which order you put
the directives.

In fact, I bet that the code looks for the "last instance of the string 'using'"
and strips everything after that (which will work with just one of the two).

[tylermitchell@…]

Is this being fixed?  I've seen other people having problems too.

[akrherz@…]

*** Bug 1544 has been marked as a duplicate of this bug. ***

[akrherz@…]

--- mappostgis.c-old    2005-11-27 22:53:13.000000000 -0600
+++ mappostgis.c        2005-11-27 22:53:22.000000000 -0600
@@ -1837,7 +1837,7 @@

     /* this is a little hack so the rest of the code works.  If the ' using
SRID=' comes before */
     /* the ' using unique ', then make sure pos_opt points to where the ' using
SRID' starts! */
-    pos_opt = (pos_srid > pos_urid) ? pos_srid : pos_urid;
+    pos_opt = (pos_srid > pos_urid) ? pos_urid : pos_srid;

     /* Scan for the table or sub-select clause */
     pos_scn = strstrIgnoreCase(data, " from ");

[dmorissette]

Paul, Jerry, can someone please look into this? I'm not setup to work with
postgis, but apparently this bug causes all POSTGIS layers that use subselects
to fail on 4.8b2

[jerry.pisk@…]

I'll take a look at this, I have the resources to test this. But it may be few 
days before I can get to it timewise.

[kenlord@…]

Just to confirm that this bug is more than just change in how OID's are handled 
in PostgreSQL 8.1, This bug affects me using PostGIS 1.0.4 and PostgreSQL 8.0.3 
and does appear to be acting in the manner described by the other posters to 
this bug report.

Here's my DATA string:

DATA "wkb_geometry FROM (SELECT * FROM my_funky_table) AS foo USING
UNIQUE ogc_fid USING SRID=26910"

And here's the error:

msDrawMap(): Image handling error. Failed to draw layer named 'Funky_Table'.
prepare_database(): Query error. Error executing POSTGIS DECLARE (the
actual query) statement: 'DECLARE mycursor BINARY CURSOR FOR SELECT
text::text,asbinary(force_collection(force_2d
(wkb_geometry)),'NDR'),ogc_fid::text
from (SELECT * FROM my_funky_table) AS foo USING UNIQUE ogc_fid WHERE
wkb_geometry && setSRID('BOX3D(499573.999999999
5463794,499664.000000001 5463866)'::BOX3D, 26910 )'

Postgresql reports the error as 'ERROR: syntax error at or near
"USING" at character 169 '

-- If I remove the 'USING UNIQUE ogc_fid', leaving only the 'USING SRID=26910' 
in my data string, MapServer appears to be defaulting to using the OID column 
in its place, In this case I receive the error that the OID column does not 
exist ... which of course it does in postgreSQL 8.0.3.

Thanks,
Ken Lord

[sgillies@…]

I'll pick this one up and apply Daryl's patch.

[sgillies@…]

I wrote 3 new Python mapscript tests (pgtest.py) to cover these cases:

- straight postgis table
- a view with using unique and using srid
- a sub-select using unique and using srid

Daryl's patch seems to get the last two cases working again. In case you want to
run these tests, see the comments inside mapserver/tests/makefile_postgis.
Committed to CVS head, and waiting for confirmation before I close this bug.

[frequens@…]

from my perspective (Linux 64, php-mapscript and cgi version) it looks that the
CVS code has fixed the 1443 trouble.
Postgis queries/views with subselects execute successfully/normally as in 4.6x
series.

[thomas.bonfort@…]

I don't think this should be closed yet.
I'm still getting errors on these two statements with CVS head (those queries
work with 4.6 and 4.8beta1):

DATA "the_geom from 
	(SELECT gid, the_geom, (case when nature='Chemin' then 2 else 1 end) AS ll FROM
troncon_chemin)
	as foo using unique gid"

and

DATA "the_geom from 
	(SELECT gid, the_geom, (case when largeur<5 then 1 else 3 end) AS ll FROM
troncon_route)
	as foo using unique gid"


the error message is 

Warning: [MapServer Error]: prepare_database(): Error executing POSTGIS DECLARE
(the actual query) statement: 'DECLARE mycursor BINARY CURSOR FOR SELECT
ll::text,asbinary(force_collection(force_2d(the_geom)),'NDR'),gid::text from
(SELECT gid, the_geom, (case when nature='Chemin' then 2 else 1 end) AS ll FROM
troncon_chemin) as foo using unique gid WHERE the_geom && setSRID('BOX3D(864000
2036997,877900 2048673)'::BOX3D, find_srid('','troncon_chemin','the_geom') )'

Postgresql reports the error as 'ERROR: syntax error at or near "using" at
character 226 '

[sgillies@…]

I made the only change to mappostgis.c since the first beta. See 
http://cvs.gis.umn.edu/cgi-bin/viewcvs/mapserver/mappostgis.c. Are you sure your
selections were working in 4.8-beta1? It also looks to me like you are failing
to provide an SRID in these selections.

[thomas.bonfort@…]

ok, checked using cvs head but mappostgis.c from 4.8beat1, and it works again.
The only change between the two files is the pos_srid : pos_urid stuff.
why should the srid be mandatory?

[sgillies@…]

Tom, you are right, there remains a problem. The postgis connector is failing to
eliminate "using unique gid" from the query string. I'll try to wrap this up
tomorrow morning.

[akrherz@…]

Yeah, I see the problem here.  pos_srid is -1, since no SRID is defined.  Lets
try this again

--- mappostgis.c-save   2005-12-13 21:57:00.000000000 -0600
+++ mappostgis.c        2005-12-13 22:15:59.000000000 -0600
@@ -1840,7 +1840,9 @@

     /* this is a little hack so the rest of the code works.  If the ' using
SRID=' comes before */
     /* the ' using unique ', then make sure pos_opt points to where the ' using
SRID' starts! */
-    pos_opt = (pos_srid > pos_urid) ? pos_urid : pos_srid;
+    if (!pos_srid && !pos_urid){ pos_opt = pos_urid;}
+    else if (pos_srid && pos_urid){pos_opt = (pos_srid > pos_urid)? pos_urid :
pos_srid;}
+    else { pos_opt = (pos_srid > pos_urid) ? pos_srid : pos_urid; }

     /* Scan for the table or sub-select clause */
     pos_scn = strstrIgnoreCase(data, " from ");


We have 5 possibilities. 

  pos_srid  pos_urid  pos_opt
  -1        -1        -1       # covered in ifelse 1
  90        89        89       # covered in ifelse 2, pick smallest val
  89        90        89       # covered in ifelse 2, pick smallest val
  89        -1        89       # covered in ifelse 3, pick largest val
  -1        89        89       # covered in ifelse 3, pick largest val

Wanna give that a whirl?  I think it works, but I thought my last patch worked
too :(

[thomas.bonfort@…]

works for me in the test case I submitted before (without srid, that is)
thanks

[sgillies@…]

I wrote a test for the case of a selection without SRID definition and Daryl's
patch lets this test pass. I think we're good to go. Changes committed to
mappostgis.c rev 1.65.