Opened 6 years ago

Closed 6 years ago

#2200 closed enhancement (fixed)

[raster] [raster] ST_Union(rast, uniontype) should be deprecated or support multiple bands

Reported by: robe Owned by: Bborie Park
Priority: medium Milestone: PostGIS 2.1.0
Component: raster Version: trunk
Keywords: history Cc:

Description

I was all about to put in an enhancement ticket because of my experience with #2199 until I realized my big beef is with ST_Union(rast,uniontype). The other functions behave as expected without explanation: e.g. they either take the band they specify or they take all bands.

so my proposal is to either mark ST_Union(rast, uniontype) as deprecated and remove it from description as at top (just remove the proto from our documentation) or have it take all bands.

The reason I say this is I realized, if I wanted to work with multiple bands, I'd do this:

(WITH mset AS 
( SELECT ROW_NUMBER() OVER() As rnum, rast
  FROM (SELECT  ST_Tile(rast, (ST_Width(rast))::integer/2, ST_Height(rast)/2) As rast 
FROM (SELECT ST_Resize(rast,0.10,0.10) As rast
FROM pics 
WHERE filename='Mona_Lisa.jpg' LIMIT 1) As  foo
) As foofoo )
SELECT ST_Union(ST_SetUpperLeft(rast,ST_UpperLeftX(rast)*-1,ST_UpperLeftY(rast)*-1)
  , ARRAY[ROW(1, 'MEAN'), ROW(2, 'MEAN'), ROW(3, 'MEAN')]::unionarg[] )
FROM mset where rnum IN(1,2))

And that works as expected -- working with all bands and if I had multiple bands -- I would assume as I did in #2199 and be sorely disappointed.

I think we also neglected to mention unionarg variant is a new variant in 2.1. I'll update the docs for that one.

So why do I want to get rid of ST_Union(rast,uniontype) -- because it violates the principle of least surprise and its yet another proto to remember that forces me to write the word EXCEPT.

Note: I'm not proposing we remove it because all those 1 band folks might be using it. Just suggesting we don't document it and if we do keep document it (and not deprecate it), change it to behave like the other functions so it does not violate the principle of least surprise.

Change History (4)

comment:1 Changed 6 years ago by Bborie Park

Status: newassigned

It makes sense that ST_Union(raster) = ST_Union(raster, 'LAST'). But that only makes sense when the union type is 'LAST'. Any other union type for ST_Union(raster, uniontype) and the behavior is unknown/undefined/???.

+1 for deprecating ST_Union(raster, uniontype). I'll change the docs.

comment:2 Changed 6 years ago by robe

Why would ST_Union(raster, any_other_type) be undefined?

To me it just means apply the same union operation to every band or am I missing something like if you are blending time series

comment:3 Changed 6 years ago by Bborie Park

I suspect I'll end up just adding support for all bands for ST_Union(raster, uniontype). I'll see when I can...

comment:4 Changed 6 years ago by Bborie Park

Keywords: history added
Resolution: fixed
Status: assignedclosed

ST_Union(raster, uniontype) supports all bands of all input rasters as of r11207.

Note: See TracTickets for help on using tickets.