Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#2454 closed defect (fixed)

[raster] ST_PixelAsPolygons does not obey exclude_nodata_value=TRUE

Reported by: alfred Owned by: Bborie Park
Priority: medium Milestone: PostGIS 2.1.1
Component: raster Version: 2.1.x
Keywords: history Cc:

Description

ST_PixelAsPolygons and all related functions (Ex: ST_PixelAsCentroids) always iterate over all pixels, even the ones with nodatavalue. Setting the "exclude_nodata_value" parameter does not seem to affect the result at all.

Seen with PostGIS 2.1.0 final. Patch for this will be greatly appreciated.

Change History (16)

comment:1 by Bborie Park, 11 years ago

When exclude_nodata_value = true, the value for NODATA pixels are returned as NULL. When exclude_nodata_value = false, the value for NODATA pixels are returned as the number designated the NODATA value.

You can get rid of those NULL values by adding a where clause…

SELECT (gv).x, (gv).y, (gv).val, ST_AsText((gv).geom) geom
FROM (SELECT ST_PixelAsPolygons(
                 ST_SetValue(ST_SetValue(ST_AddBand(ST_MakeEmptyRaster(2, 2, 0, 0, 0.001, -0.001, 0.001, 0.001, 4269), 
                                                    '8BUI'::text, 1, 0), 
                                         2, 2, 10), 
                             1, 1, NULL), 1, true
) gv 
) foo
WHERE (gv).val IS NOT NULL

comment:2 by Bborie Park, 11 years ago

Looking at how the behavior changed between 2.0 and 2.1 (the exclude_nodata_value parameter was added in 2.1), ST_PixelAsPolygons() has always returned NULL value for a pixel set to the NODATA value. This behavior is present before 2.0.

The exclude_nodata_value parameter was added in 2.1 so that ALL pixel values would be returned regardless of a pixel being NODATA or not.

If people think that the behavior should be changed so that when exclude_nodata_value = true NODATA pixels are not returned, that change can be made for 2.2.

comment:3 by pracine, 11 years ago

There are three options here:

1-We don't want NODATA pixels to be returned as polygons

2-We want them to be returned but set to NULL

3-We want them to be returned but set as the NODATA value (e.g. 0), not NULL

Option 3 should be possible by deactivating the nodatavalue of the raster with ST_SetBandNoDataValue(rast, null).

Option 1 and 2 should be controled by exclude_nodata_value and this is the only thing that exclude_nodata_value should control.

Is this the current behavior?

comment:4 by Bborie Park, 11 years ago

In 2.0, only option 2 was available. In 2.1, options e 2 and 3 are possible.

Changing the behavior to be options 1 and 2 can be done for 2.2. I don't believe we can do the same for 2.1 as I don't consider this a bug as the default behavior in 2.1 matches that of 2.0.

comment:5 by pracine, 11 years ago

What about exclude_nodata_value in other functions like ST_DumpAsPolygons()? Does it follow this behavior or same as ST_PixelAsPolygons()?

comment:6 by Bborie Park, 11 years ago

In most functions, exclude_nodata_value does as the parameter name implies. In 2.0, ST_DumpAsPolygons() didn't have the exclude_nodata_value parameter. It always excluded NODATA pixels.

ST_PixelAsPolygons() always included NODATA pixels in the output…

comment:7 by alfred, 11 years ago

I have been using this workaround for now, by checking if the value is equal to the NODATA value.

In my case i have rasters where only small portions contain data and i am attempting to iterate the areas that have data and set values.

The problem with that is that it increases the query time significantly by going over all the pixels (can be thousands of those), even that i care only about the ones that have data only.

I was hoping that now that this function is implemented in C that this will provide boost in the speed, which it seems that it does in general, but not for this case.

I still think that having the "exlude_nodata_value" , should NOT return those pixels.

Having it set as false should return all the pixels and the values can still be checked with a WHERE clause. I don't see much benefit of having it set to NULL as in any case there needs to be a WHERE clause and the same can checked for being equal to ST_BandNoDataValue , but may be nice option in some cases.

comment:8 by Bborie Park, 11 years ago

Milestone: PostGIS 2.1.1PostGIS Future
Status: newassigned
Version: 2.1.xtrunk

I'll change the behavior for 2.2 unless robe indicates I can make the change for 2.1.

in reply to:  7 ; comment:9 by Bborie Park, 11 years ago

The problem with that is that it increases the query time significantly by going over all the pixels (can be thousands of those), even that i care only about the ones that have data only.

It will always go over every pixel. How else are you to know what the value of a pixel is?

I was hoping that now that this function is implemented in C that this will provide boost in the speed, which it seems that it does in general, but not for this case.

It was always in C, even in 2.0.

in reply to:  9 comment:10 by alfred, 11 years ago

Replying to dustymugs:

The problem with that is that it increases the query time significantly by going over all the pixels (can be thousands of those), even that i care only about the ones that have data only.

It will always go over every pixel. How else are you to know what the value of a pixel is?

I was hoping that now that this function is implemented in C that this will provide boost in the speed, which it seems that it does in general, but not for this case.

It was always in C, even in 2.0.

I am sorry , what i mean is that if you do not believe that this check done in C is going to be faster than doing a SQL WHERE check, then probably there is no point in having this parameter (exclude_nodata_value).

comment:11 by Bborie Park, 11 years ago

My bad. ST_PixelAsPolygons was a pl/pgsql function in 2.0.

comment:12 by Bborie Park, 11 years ago

Milestone: PostGIS FuturePostGIS 2.2.0

comment:13 by Bborie Park, 11 years ago

Milestone: PostGIS 2.2.0PostGIS 2.1.1
Version: trunk2.1.x

After chatting with robe2 on IRC, the decision was made to treat this as a bug. So, 2.1.1 it is.

comment:14 by Bborie Park, 11 years ago

Keywords: history added
Resolution: fixed
Status: assignedclosed

Fixed at r11897 for 2.1. Fixed at r11898 for 2.2

comment:15 by alfred, 11 years ago

Thank you! That is now an exponential increase in speed!

comment:16 by robe, 11 years ago

exponential! — how exponential are we talking about?

Note: See TracTickets for help on using tickets.