Opened 10 years ago
Closed 10 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|
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 by , 10 years ago
|Status:||new → assigned|
comment:2 by , 10 years ago
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 by , 10 years ago
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 by , 10 years ago
|Status:||assigned → closed|
ST_Union(raster, uniontype) supports all bands of all input rasters as of r11207.
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.