[raster] Make ST_Union a c-implementation

[robe]

Bborie,

Just putting this on the list before I forget to. I'm still revising Pierre's code to get rid or the rasterexp and the infinite number of state functions. I'm almost done but doing some spot checking to make sure my revised looks right.

Hopefully that will make it easier to rewrite when the time comes since there won't be quite so many moving parts.

I was tempted to make it multiband supportable, as now I have to write stupid code like this to reconstitute my chopped pictures and which was even more painful without the array ST_AddBand construct.

SELECT ST_AddBand(NULL, 
ARRAY[ST_Union(ST_Band(rast,1))
    , ST_Union(ST_Band(rast,2))
    , ST_Union(ST_Band(rast,3)) ] )
FROM samples.downtown_chunked 

[robe]

Actually we should probably think about how the ST_Union interface will change when we introduce multiband support since if a lot of people start using a particular construct, we can't just pull it out.

I would have liked ST_Union(rast) to just union bands in parallel like I have above or an argument to denote that is the behavior you want with an array of numbers like ST_Band. Perhabs something like a

ST_Union(rast raster,band_nums integer[])

But Pierre has 7 ST_Union aggregate permutations already none of which take a band number.

[dustymugs]

Multiband ST_Union would be nice but we're not going to have significant performance gains until I rewrite the C 2-raster MapAlgebra.

7 permutations without a band parameter? Does all versions work on all bands?

[robe]

No all permutations as far as I can tell assumes the first band is what you want to work with. Actually I haven't tested his other permutations. i was mostly testing with ST_Union(rast) and ST_Union(rast, p_expression) — the first just defaults to 'LAST' I think.

I'm almost tempted to cut the rest out and just leave maybe two since I think the most common use case for ST_Union is going to be stitching back your chunked rasters based on a spatial filter or grouping which is pretty much satisfied by the first two. Then leave the other permutations up for discussion. I don't think Pierre would like me stripping out 5 of his functions (which of course is more like 20 functions). If I keep all 7, I think I can do it still without introducing the rastexpr and cutting out most of his various permutations of state functions. So it will still be a savings of about 10 functions.

[robe]

By grouping I meant something like a file_name that raster2pgsql currently offers. That way you can stitch back a chunked dtem tile or something. That I see is the number 2 case next to spatial aggregation. I think all these extra permutations are going to confuse the hell out of people. With that said would be nice to see one version that takes a band number so you don't have to do ST_Band and alter expand that to take an array of bands.

[dustymugs]

I'm assuming that some of the permutations are duplicates of others as I don't believe Pierre uses default parameter values.

[robe]

Yah those I took care of to some extent, but AGGREGATES don't seem to support default args fully (you can't define a default arg withint a CREATE AGGREGATE nad the state functions it doesn't seem to want to use one with a default arg that would otherwise match what it needs), so you would end up with more than you normally would with a regular function.

[dustymugs]

I've broken this ticket into two due to the two distinct goals. Refer to #2021 for multi-band support in ST_Union.

[dustymugs]

Added in r10344. This C-implementation won't be backported to PostGIS 2.0 as it isn't a bug fix AND the significant amount of underlying code added/removed.

[robe]

cool can't wait to try this out to compare with my previous workloads

[pracine]

Great work Bborie!

Did you test performance?

I think the key question is "Is the state function still iterating on all the pixels forming the union of the raster or just on the changing extent?"

[dustymugs]

It runs on whatever the union's evolving extent needs to be.

[pracine]

So we should expect O(n^{2) for a coverage of disjoint tiles? As demonstrated in my blog post? Not much change. Just that C is (supposed to be) faster.}

[dustymugs]

In addition to non-stop deserialization and hexwkb encoding/decoding…

[pracine]

Do you mean performance is bad?

[dustymugs]

No. Performance is better because of all the deserialization and hexwkb encoding/decoding that happens to the working union raster with every call to the function doesn't happen anymore with the C variant.

[pracine]

Ok. But still, time processing should grow by a second degree order with the number of pixel involved?