[postgis-users] operator is not unique: text || geometry

[Rémi Cura]

Hey Stephan,

I'm afraid I must say the coding style is not good and dangerous.
And I know that the very nature of plpgsql langage (and doc) make it
difficult to produce a nice function.

AS it is, it is very hard to read it, and any user of your function could
potentially inject SQL via your function.

For instance,
----------
  execute 'CREATE TABLE ' || updatedtablename || ' AS SELECT objectid,
f_code, shape  FROM ' || tablename;
------------
should become (safe, easier to read, easier to port)
-----------
execute format('CREATE TABLE %I AS SELECT objectid, f_code, shape  FROM %I
; ', updatedtablename,tablename) ;
-----------

AS a rule of thumb, you should start asking yourself question when you
abuse quoting, like ~'''~

For instance
-------------
execute 'SELECT topology.CreateTopology(''' || cleantopo || ''',32648,
0.000001, TRUE);';
-------------
could simply be replaced by
---------------------
PERFORM topology.CreateTopology(clean_topo,32648, 0.000001, TRUE) ;
---------------------
or, if you really really want to use an EXECUTE :
---------------------
EXECUTE 'SELECT topology.CreateTopology(%s,32648, 0.000001, TRUE) ;
',clean_topo ) ;
---------------------

You might also use the type 'regclass' instead of the type text when the
text shall always represent a table.
This would automatically raise error if the table doesn't exist, and would
be schema-qualification safe.


Now to the supposed-to-be-faulty part, you have a sql problem on top of
plpgsql problem.
I would recommend to always fabricate your sql statement, then test it
manually, then execute it.
for instance, you could declare
_q text;
then you fabricate your query :
---------------
_q :=
'UPDATE fgcm.'||updatedtablename||' SET '||topo_shape||'::topogeometry
             =
topology.toTopoGeom(ST_Transform('||r.shape||'::geometry,32648),
'||cleantopo||', 1, 1.0)
            WHERE '||objectid||' = '||r.objectid||';';
RAISE EXCEPTION 'here is the query to manually test : %',_q ;
-------------------
then you print _q and execute it manually to see if the sql syntax is
correct :
(copy past the query given at execution time, then try to execute it in
pgadmin or psql to check that syntax is correct).

The problem in this query is that it doesn't respect the SQL UPDATE syntax :
you should do something like :
UPDATE your_table_name SET (-*list_of_columns_to_update*-) =
(expression_matching_list_of_columns) WHERE ...

So you can see that "SET '||topo_shape||'::topogeometry" is not correct.
It is the same for your WHERE part :
"WHERE '||objectid||' = '||r.objectid||';';"

Now it could be a feature of your code (like storing the name of columns to
use in another table), I don't understand it sufficiently to say so.

Cheers,
Rémi-C

2015-02-17 10:04 GMT+01:00 Sandro Santilli <strk at keybit.net>:

> On Mon, Feb 16, 2015 at 10:02:14PM +0000, Miller, Stephan wrote:
>
> > I highlighted where I think the error is occurring in red below.
>
> Please, *please*, don't use colors to highlight texts.
> I know we're in 2015 and the internet is full of colorful moving puppets,
> but if we stick to low tech it's easier for everyone to partecipate.
>
> Personally, I don't see colors in emails.
>
> --strk;
> _______________________________________________
> postgis-users mailing list
> postgis-users at lists.osgeo.org
> http://lists.osgeo.org/cgi-bin/mailman/listinfo/postgis-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/postgis-users/attachments/20150217/40a01db2/attachment.html>