headers
ivan minčík | 2 Dec 2007 22:58
Picon

Re: where to register to submit bug?

Thanks, I hope it will help to others...

Mark Cave-Ayland wrote:
> On Sat, 2007-11-17 at 20:34 +0000, Mark Cave-Ayland wrote:
>
>   
>> So the real bug in this case is that the (E)WKB parser doesn't seem to
>> attempt to validate any geometries being passed through it :(  I've had
>> a look at the parser code and it's quite a complicated beast with not
>> many comments, so I imagine a fix is going to a little time to come up
>> with.
>>     
>
> FWIW I've now committed a patch to SVN to include simple validation for
> geometries being input in (E)WKB format. From latest SVN:
>
>
> bug=# select
> astext('010600002011080000040000000103000000010000000400000052B81E855E5914C100000040E96133C1EC51B81EA35914C1E17A146EE76133C18FC2F5289E5914C114AE47E1D76133C15C8FC2F58D5914C114AE4761C16133C10103000000010000000500000066666666805914C11F85EB51DB6133C133333333835914C11F85EB51E46133C166666666645914C152B81E05E56133C100000000625914C1EC51B81EDC6133C166666666805914C11F85EB51DB6133C10103000000010000000100000048E17A14555914C166666666C26133C1010300000001000000010000001F85EB51565914C1EC51B85EC66133C1'::geometry);
> ERROR:  geometry contains non-closed rings
> bug=#
>
>
> Hopefully this means future releases will prevent these ill-formed
> geometries getting into the database in the first place when inserted as
> (E)WKB.
>
>
> Kind regards,
>
(Continue reading)

Permalink | Reply |
headers
Dave Fuhry | 14 Jun 2008 20:15
Picon

Re: where to register to submit bug?

Mark,

   I'm beginning to wonder if the stricter-EWKB-parsing patch applied
in November was a mistake.

   I have an app which bulk-loads shapefiles (of varying quality),
then "repairs" or NULLs geometries which are not isvalid().  I'm not
finding a good way to bulk-load input data when the dataset has a
record which causes:

ERROR:  geometry contains non-closed rings

COPY (shp2pgsql -D) is out, since COPY aborts on error.  From
discussions on pgsql-dev, it is not clear whether COPY will support a
"SKIP ERRORS" or "ERRORS TO error_table" clause anytime soon.  Even in
that case, I would like a convenient way to keep the table's other
(non-geometry) attributes.

For shp2pgsql's insert-statement mode, records are grouped into
250-record batches surrounded by BEGIN; ... END;, so an erroneous
record will abort the 250 records in its batch.  Removing transactions
entirely is no good for bulk-loading, since the database will be
forced to commit every record to disk before processing the next.

Another option would be to move EWKB parsing logic to shp2pgsql so
that shp2pgsql can decide how to handle erroneous geometries.  This
option seems ugly and redundant to me, although I'll defer judgement.

Lastly, maybe some per-session option to allow postgis to import
erroneous geometries is in order.  Then they can be corrected in a
(Continue reading)

Permalink | Reply |
headers
Mark Cave-Ayland | 19 Jun 2008 12:43
Picon

Re: where to register to submit bug?

Dave Fuhry wrote:
> Mark,
> 
>    I'm beginning to wonder if the stricter-EWKB-parsing patch applied
> in November was a mistake.
> 
>    I have an app which bulk-loads shapefiles (of varying quality),
> then "repairs" or NULLs geometries which are not isvalid().  I'm not
> finding a good way to bulk-load input data when the dataset has a
> record which causes:
> 
> ERROR:  geometry contains non-closed rings
> 
> COPY (shp2pgsql -D) is out, since COPY aborts on error.  From
> discussions on pgsql-dev, it is not clear whether COPY will support a
> "SKIP ERRORS" or "ERRORS TO error_table" clause anytime soon.  Even in
> that case, I would like a convenient way to keep the table's other
> (non-geometry) attributes.
> 
> For shp2pgsql's insert-statement mode, records are grouped into
> 250-record batches surrounded by BEGIN; ... END;, so an erroneous
> record will abort the 250 records in its batch.  Removing transactions
> entirely is no good for bulk-loading, since the database will be
> forced to commit every record to disk before processing the next.
> 
> Another option would be to move EWKB parsing logic to shp2pgsql so
> that shp2pgsql can decide how to handle erroneous geometries.  This
> option seems ugly and redundant to me, although I'll defer judgement.
> 
> Lastly, maybe some per-session option to allow postgis to import
(Continue reading)

Permalink | Reply |
headers
Stephen Woodbridge | 19 Jun 2008 15:42
Favicon
Gravatar

Re: where to register to submit bug?

Mark Cave-Ayland wrote:
> Dave Fuhry wrote:
>> Mark,
>>
>>    I'm beginning to wonder if the stricter-EWKB-parsing patch applied
>> in November was a mistake.
>>
>>    I have an app which bulk-loads shapefiles (of varying quality),
>> then "repairs" or NULLs geometries which are not isvalid().  I'm not
>> finding a good way to bulk-load input data when the dataset has a
>> record which causes:
>>
>> ERROR:  geometry contains non-closed rings
>>
>> COPY (shp2pgsql -D) is out, since COPY aborts on error.  From
>> discussions on pgsql-dev, it is not clear whether COPY will support a
>> "SKIP ERRORS" or "ERRORS TO error_table" clause anytime soon.  Even in
>> that case, I would like a convenient way to keep the table's other
>> (non-geometry) attributes.
>>
>> For shp2pgsql's insert-statement mode, records are grouped into
>> 250-record batches surrounded by BEGIN; ... END;, so an erroneous
>> record will abort the 250 records in its batch.  Removing transactions
>> entirely is no good for bulk-loading, since the database will be
>> forced to commit every record to disk before processing the next.
>>
>> Another option would be to move EWKB parsing logic to shp2pgsql so
>> that shp2pgsql can decide how to handle erroneous geometries.  This
>> option seems ugly and redundant to me, although I'll defer judgement.
>>
(Continue reading)

Permalink | Reply |
headers
Paul Ramsey | 20 Jun 2008 10:24
Picon
Gravatar

Re: where to register to submit bug?

This is a well-chewed issue, and we have come down repeatedly in
favour of allowing invalid geometry to be loaded. Which, I suppose,
means we should loosen the parser to allow even non-closed rings.

What *does* need to be done (issue for me, it's not that hard) is a
few additional hooks to the GEOS isvalid routines so people can easily
identify WHY and WHERE their geometries are invalid.

More difficult, probably need in of some funding to get Martin's
attention, is some formalized cleaning routines that go beyond what
Buffer(0) can accomplish.

P.

On Thu, Jun 19, 2008 at 3:42 PM, Stephen Woodbridge
<woodbri <at> swoodbridge.com> wrote:
> Mark Cave-Ayland wrote:
>>
>> Dave Fuhry wrote:
>>>
>>> Mark,
>>>
>>>   I'm beginning to wonder if the stricter-EWKB-parsing patch applied
>>> in November was a mistake.
>>>
>>>   I have an app which bulk-loads shapefiles (of varying quality),
>>> then "repairs" or NULLs geometries which are not isvalid().  I'm not
>>> finding a good way to bulk-load input data when the dataset has a
>>> record which causes:
>>>
(Continue reading)

Permalink | Reply |
headers
Mark Cave-Ayland | 20 Jun 2008 11:17
Picon

Re: where to register to submit bug?

Paul Ramsey wrote:
> This is a well-chewed issue, and we have come down repeatedly in
> favour of allowing invalid geometry to be loaded. Which, I suppose,
> means we should loosen the parser to allow even non-closed rings.

Dropping the checks from the parser is a relatively straightforward 
thing to do. My concern would be that some of the basic routines (e.g. 
area calculations) would start to fail if rings weren't closed :(

> What *does* need to be done (issue for me, it's not that hard) is a
> few additional hooks to the GEOS isvalid routines so people can easily
> identify WHY and WHERE their geometries are invalid.

Yeah, that would be quite useful to have. Is this something that would 
require work on GEOS before the functionality could be added to PostGIS?

> More difficult, probably need in of some funding to get Martin's
> attention, is some formalized cleaning routines that go beyond what
> Buffer(0) can accomplish.

There does seem to be a lot of discussion recently about new features; 
perhaps we need to add a "How to sponsor a feature" section to the wiki 
explaining how companies can approach developers to get new features 
implemented...

ATB,

Mark.

--

--
(Continue reading)

Permalink | Reply |

Gmane