Opened 9 years ago

Closed 9 years ago

Last modified 8 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 by strk, 9 years ago

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 by Bborie Park, 9 years ago

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 by strk, 9 years ago

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 by robe, 9 years ago

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 by strk, 9 years ago

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 by Bborie Park, 9 years ago

Keywords: history added

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

comment:7 by Bborie Park, 9 years ago

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 by strk, 9 years ago

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 by Bborie Park, 9 years ago

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 by robe, 9 years ago

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

comment:11 by strk, 8 years ago

See #3039 for a issues deriving from this work

comment:12 by strk, 8 years ago

Basically we forgot to revert r12101

comment:13 by strk, 8 years ago

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.