Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#2532 closed defect (fixed)

Unexisting commutator operators for raster

Reported by: strk Owned by: Bborie Park
Priority: medium Milestone: PostGIS 2.0.5
Component: raster Version: 2.0.x
Keywords: history Cc:

Description

Operator ~ (geometry, raster) defines @ as the commutator, but there is no operator @ (raster, geometry) defined.

Operator ~ (raster, geometry) defines @ as the commutator, but there is no operator @ (geometry, raster) defined.

Those operators seem to be implicitly defined by PostgreSQL, due to the COMMUTATOR specification, but the script analyzing the .SQL fails to realize such an object exists and thus fails to drop/upgrade/add-to-extension (depending on the script output).

If the script is the only issue I guess I could improve the script to deal with the case, but I'm not sure if there are other consequences.

Change History (13)

comment:1 Changed 6 years ago by strk

Alright it turns out it's really a bug, in that the COMMUTATOR spec only defines a shell:

pg22=# select null::raster @ null::geometry;
ERROR:  operator is only a shell: raster @ geometry

I'm going to drop the COMMUTATOR spec in order to go on with #2531

comment:2 Changed 6 years ago by Bborie Park

Milestone: PostGIS 2.1.1PostGIS 2.0.5
Status: newassigned

It looks like those "one-sided" operators were added in r9372. I'll add what's missing.

comment:3 Changed 6 years ago by strk

I've already committed a drop in trunk with r12101, to be ported to 2.1 and 2.0 branches. For trunk it's surely ok to add the missing parts, but I'm not sure it's ok to do so in the stable branches.

comment:4 Changed 6 years ago by robe

We can't fix operators without dropping them and I don't think we can drop them without dropping the table indexes. Probably not something we should rush into with 2.1 and 2.0.

comment:5 Changed 6 years ago by strk

Ouch, so the only _safe_ way is to _add_ the operators, right ? @dustymugs, better do the addition as you planned at this point, start from trunk and we see where we get to. I shall notice that the change in operator didnt' trigger _any_ failure in the testsuite, which is scary. I guess the outcome is that upgrades are NOT correct but there's no sign of it, even if postgis_full_version() doesnt' warn you about it.

comment:6 Changed 6 years ago by Bborie Park

Keywords: history added

Fixed in -2.0 in r12106, -2.1 in r12107. -trunk in r12018.

comment:7 Changed 6 years ago by Bborie Park

Resolution: fixed
Status: assignedclosed

Last fix for 2.0 on 8.4 in r12109. Also, revision for -trunk is wrong. It should be r12108

comment:8 Changed 6 years ago by strk

I wonder if the add_missing_operator function should be generalized to be accessible to all upgrade scripts. Ie: isn't this a job for util/postgis_proc_upgrade.pl ?

comment:9 Changed 6 years ago by Bborie Park

I don't think it's absolutely necessary. This is just because 8.4 has special needs. It would be nice if the upgrade scripts were wrapped in something (DO or function) so that we can add logic.

comment:10 Changed 6 years ago by robe

well you only need to worry about 8.4 for PostGIS 2.0. 2.1 9.0 is the minimum.

comment:11 Changed 4 years ago by strk

See #3039 for a issues deriving from this work

comment:12 Changed 4 years ago by strk

Basically we forgot to revert r12101

comment:13 Changed 4 years ago by strk

For the record, I made some research and found that:

  1. The new operators (needed for COMMUTATOR) first appeared added in 2.0.5 and 2.1.1
  2. The commenting-out of COMMUTATOR first appeared in 2.1.2 due to r12101 never being reverted

As a consequence:

  1. The only correct version so far is 2.1.1
  2. Whoever installed PostGIS from 2.1.2 up to 2.1.5 have un-commutable operators raster~geometry and geometry~raster

Right now it would take a dump-reload to re-add the now-missing COMMUTATOR in the existing operators

Note: See TracTickets for help on using tickets.