Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#5546 closed defect (wontfix)

ERROR: operator is not unique: topogeometry <> topogeometry

Reported by: strk Owned by: strk
Priority: blocker Milestone: PostGIS 3.5.0
Component: topology Version: master
Keywords: regression Cc:

Description

Something must have changed in current master branch of PostGIS as the topogeometry ≠ topogeometry operator is not unique anymore.

This was spotted in a regress testsuite of downstream topo_update project: https://gitlab.com/nibioopensource/pgtopo_update_sql/-/issues/262

Change History (10)

comment:1 by strk, 8 months ago

PostGIS 3.4.0-28-g59654b1aa does not have this problem

comment:2 by strk, 8 months ago

Possible path to follow: #5175

comment:3 by strk, 8 months ago

Component: postgistopology
Keywords: regression added
Priority: mediumblocker

The code triggering this error is a plpgsql snippet (https://gitlab.com/nibioopensource/pgtopo_update_sql/-/issues/262#note_1571809528):

IF rec2.old_topogeom != newBorder THEN

Both rec2.old_topogeom and newBorder are of type topology.TopoGeometry.

That construct worked just fine in postgis-3.4 and fails now, so this is a regression

comment:4 by strk, 8 months ago

Reverting [71fefd690fbc887a7399518ea56f17797faf1c11/git] fixes this regression, but an equality test would still yeld: ERROR: operator is not unique: topogeometry = topogeometry so I guess we'd better define explicit operators for the TopoGeometry type too.

comment:5 by strk, 8 months ago

I've tried dropping the cast from topogeometry to integer[] so that the only implicit cast is toward geometry:

pg35=# \dC topogeometry
                   List of casts
 Source type  | Target type | Function | Implicit?
--------------+-------------+----------+-----------
 topogeometry | geometry    | geometry | yes
(1 row)

Still, topogeometry <> topogeometry is reported as being ambiguous, while geometry <> geometry is not:

pg35=# select 'POINT EMPTY'::geometry <> 'POINT EMPTY'::geometry;
f
pg35=# select (1,1,1,1)::topogeometry <> (1,1,1,1)::topogeometry;
ERROR:  operator is not unique: topogeometry <> topogeometry

comment:6 by strk, 8 months ago

Dropping the operator <> for geometry drops the ambiguity:

pg35=# alter extension postgis drop operator <> (geometry, geometry);
ALTER EXTENSION
pg35=# drop operator <> (geometry, geometry);
DROP OPERATOR
pg35=# select (1,1,1,1)::topogeometry <> (1,1,1,1)::topogeometry;
f
Last edited 8 months ago by strk (previous) (diff)

comment:7 by strk, 8 months ago

From #postgresql IRC channel on libera.chat:

11:59:43 RhodiumToad | so the basic logic is this
12:00:09 RhodiumToad | given a <> b, we first find all visible '<>' operators
12:00:36 RhodiumToad | then we see whether we can implicitly cast a and b to the argument types of those operators
12:00:54 RhodiumToad | if the resulting list contains exactly one operator, then we use that one, otherwise it's an error
12:01:14 RhodiumToad | (this is given that a and b have known types already, there's more to it if either is unknown)
12:02:34 RhodiumToad | (if there's an exact match, that'll be taken over any operator that requires a cast)
12:03:07 RhodiumToad | in this case, there are two equally good matches: record <> record and geometry <> geometry
12:03:30 RhodiumToad | since all composite types can be implicitly cast to record, and there's also an implicit cast to geometry

comment:8 by strk, 8 months ago

Resolution: wontfix
Status: assignedclosed

So the new information is that there is an implicit cast to "record", even if not shown by the \dC psql command.

This means the ambiguity is between comparing the TopoGeometry objects as if they were records or comparing them as if they were geometries. Pretty big ambiguity and it's even a much different cost (compare as geometry is going to be expensive).

The downstream code (pgtopo_update_sql) expected to compare as record, so to fix the regression we would need to add TopoGeometry inequality operator to compare the integer values of the type, but as the equality operator (=) was also ambiguous in 3.0, I'd rather keep it ambiguous for now and not consider this a regression.

comment:9 by Sandro Santilli <strk@…>, 8 months ago

In 0b2ba5f/git:

Add note about TopoGeometry inequality operator breaking change

References #5546

comment:10 by Sandro Santilli <strk@…>, 8 months ago

In 9f184d5/git:

Fix the instructions to get old TopoGeometry inequality behaviour

References #5546

Note: See TracTickets for help on using tickets.