#3006 closed defect (fixed)
[raster] Numeric overflow when executing AddRasterConstraints
Reported by: | rtorre | Owned by: | dustymugs |
---|---|---|---|
Priority: | high | Milestone: | PostGIS 2.1.6 |
Component: | raster | Version: | 2.1.x |
Keywords: | history | Cc: |
Description
We are experiencing a numeric field overflow
when adding overviews to a raster and subsequently calling AddRasterConstraints
.
Executing this:
SELECT AddRasterConstraints('cdb_importer','o_256_importer_e7efb068796911e4a19a5e0004719e63','the_raster_webmercator',TRUE,TRUE,TRUE,TRUE,TRUE,TRUE,FALSE,TRUE,TRUE,TRUE,TRUE,TRUE);
I get this in the postgresql log:
PL/pgSQL function addrasterconstraints(name,name,name,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean) line 53 at RETURN psql:/tmp/imports/20141201-2293-x4pho4/importer_e7efb068796911e4a19a5e0004719e63.sql:7472: NOTICE: SQL used for failed constraint: ALTER TABLE cdb_importer.o_256_importer_e7efb068796911e4a19a5e0004719e63 ADD CONSTRAINT enforce_sca lex_the_raster_webmercator CHECK (st_scalex(the_raster_webmercator)::numeric(16,10) = (1252219.04682188)::numeric(16,10)) CONTEXT: PL/pgSQL function _add_raster_constraint_scale(name,name,name,character) line 38 at RETURN PL/pgSQL function addrasterconstraints(name,name,name,text[]) line 60 at assignment PL/pgSQL function addrasterconstraints(name,name,name,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean,boolean) line 53 at RETURN psql:/tmp/imports/20141201-2293-x4pho4/importer_e7efb068796911e4a19a5e0004719e63.sql:7472: NOTICE: Returned error message: numeric field overflow CONTEXT: PL/pgSQL function _add_raster_constraint_scale(name,name,name,character) line 38 at RETURN
I think the problem is located in the file ./raster/rt_pg/rtpostgis.sql.in
, around lines 6964-6968:
sql := 'ALTER TABLE ' || fqtn || ' ADD CONSTRAINT ' || quote_ident(cn) || ' CHECK (st_scale' || $4 || '(' || quote_ident($3) || ')::numeric(16,10) = (' || attr || ')::numeric(16,10))'; RETURN _add_raster_constraint(cn, sql);
(See [8396] and #1373 for more on that constraint)
Executing this from psql I get the same error:
# select (1252219.04682188)::numeric(16,10); ERROR: numeric field overflow DETAIL: A field with precision 16, scale 10 must round to an absolute value less than 10^6.
I think this is related to other issues people is having, not only because of big values of st_scale
but also with (very) small values, when input values have too much precision:
- http://gis.stackexchange.com/questions/118120/numeric-issue-while-loading-floating-point-valued-geotiff-into-postgis
- http://gis.stackexchange.com/questions/59732/uploading-raster-format-tif-to-postgis-through-raster2pgsql
This file from Natural Earth can be used to reproduce the issue: http://www.naturalearthdata.com/http//www.naturalearthdata.com/download/10m/raster/SR_HR.zip
I think the input values must be rounded instead of adding a constraint.
Attachments (1)
Change History (25)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
rtorre,
I'll try to come with a better scale precision to use. The reason swe don't use round is since the raster values are stored as numeric, round doesn;t work.
So for example:
SELECT round( (1252219.04682188555)::double precision,10);
would give you an error of: ERROR: function round(double precision, integer) does not exist
So safest to just keep it as numeric cast and increase the casting scale and precision.
comment:5 by , 10 years ago
comment:6 by , 10 years ago
Since the purpose of the constraint is to check for equality of numeric
values, something like
CHECK abs(st_scalex(the_raster_webmercator) - (attr)) <= delta
should be more appropriate, with delta = 1e-10
for instance.
comment:7 by , 10 years ago
A delta
of 1e-6
should be enough to cover scale values ranging from 1e-5
through 1e14
.
I'll prepare a patch with a test with my proposal.
by , 10 years ago
Attachment: | fix_for_3006.patch added |
---|
I'm adding this patch with the hope of opening a productive discussion. I realized it breaks backwards compatibility with existing tables because the source of the constraints is used to retrieve information (see raster_columns and _raster_constraint_info_scale). Suggestions are more than welcomed.
comment:8 by , 10 years ago
rtorre — thinking about this more I think your change while in principal is fine is too much of a breaking change.
Talked with dustymugs on IRC and he was okay with just increasing the precision to 25,10.
The issue with your change is that that check constraint is parsed out by the function _raster_constraint_info_scale
So your proposed change would require us to rewrite to support that new format as well as older constraint format. Since this is a critical change that should go into 2.1.5, I don't feel comfortable doing such a change that requires such extensive testing.
comment:9 by , 10 years ago
rtorre I think we can consider your patch for 2.2.0, but not for 2.1.5. For 2.1.5 probably should just increase the precision.
comment:10 by , 10 years ago
increasing the precision to 25,10
That's good enough for me.
I think we can consider your patch for 2.2.0, but not for 2.1.5. For 2.1.5 probably should just increase the precision.
I wouldn't do much on top of functions/views that parse constraints, if for only maintainability (and possibly performance).
Thanks a lot for the prompt response.
comment:11 by , 10 years ago
increased precision to 25,10 in trunk 2.2 at r13136 — will make same change in 2.1 and then try to add your new big test regression tsst to mix and then backport to 2.1.
comment:12 by , 10 years ago
did the same fro 2.1 at r13137
Still need to commit the big raster tests
comment:14 by , 10 years ago
Summary: | Numeric overflow when executing AddRasterConstraints → [raster] Numeric overflow when executing AddRasterConstraints |
---|
comment:15 by , 10 years ago
Replying to dustymugs:
big raster tests?
I guess she's talking about the tests in the patch I uploaded. Given the change finally committed into trunk and 2.1 I do believe they are a bit overkill.
IMHO this ticket can be closed as resolved.
comment:16 by , 10 years ago
I'll close this once I have the chance to see the change affects existing databases…
comment:17 by , 10 years ago
rtorre,
I wanted to include your regress ticket. We try to test anything in regress fixed especially since you went thru the trouble of making the tests and this seemed like an important bug to regress test. I was planning to test the regress test and then commit. Should do within the next week.
comment:18 by , 10 years ago
Milestone: | PostGIS 2.1.5 → PostGIS 2.1.6 |
---|
comment:19 by , 10 years ago
Milestone: | PostGIS 2.1.6 → PostGIS 2.1.7 |
---|
comment:20 by , 10 years ago
Milestone: | PostGIS 2.1.7 → PostGIS 2.1.6 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
I'm going to take rtorre's advice and just close this. I'll open another ticket that we need big raster regress test.
comment:21 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Reopening because I think we could get this to work better…
round(ST_ScaleX(rast)::numeric, 10) == round(MYREF::numeric, 10)
comment:22 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Changed in -trunk as of r13467. I'm not going to make the same change to 2.1 as I got rid of all use of numeric(16, 10)
in favor of round(VALUE::numeric, 10)
.
comment:23 by , 10 years ago
Keywords: | history added |
---|
rtorre,
thanks, for the report.