Opened 8 years ago
Closed 8 years ago
#3665 closed defect (fixed)
Bugs in BRIN support - Index corruption and memory leak
Reported by: | Julien Rouhaud | Owned by: | robe |
---|---|---|---|
Priority: | critical | Milestone: | PostGIS 2.3.1 |
Component: | postgis | Version: | 2.3.x |
Keywords: | Cc: | rdunklau |
Description
I found two bugs in the first BRIN support implementation:
- index corruption bug. It can happen if a geometry is inserted or updated on a previously summarized range (new ranges are not automatically summuraized), and isn't contained in the stored bounding box. This bug is fixed in attached fix_brin_corruption.diff
Please note that this patch expose some required C function which were previously only used for GiST indexes.
- memory leak. The leak is actually in an existing postgis function. I believe it has been unnoticed until now because IFAICT it's only used in GiST indexes, and postgres use a per-tuple MemoryContext for such indexes. This bug is fixed in attached fix_brin_memory_leak.diff
These patches should be applied on 2.3 and trunk.
Attachments (3)
Change History (11)
by , 8 years ago
Attachment: | fix_brin_corruption.diff added |
---|
by , 8 years ago
Attachment: | fix_brin_memory_leak.diff added |
---|
comment:1 by , 8 years ago
comment:2 by , 8 years ago
rjuju,
Just checking how it's going with the regression tests. I'd like to commit them as one unit.
Thanks, Regina
comment:3 by , 8 years ago
Hello Regina,
Sorry for late answer, last week was quite busy with the pgconf.eu event. I started to add quite some new regression tests to cover all the code path I could think of, and I found another issue that we overlooked in the first implementation (mixing geometries of different number of dimension in a same block range and looking for overlapping values leads to miss the rows with the geometries containing less number of dimensions than the compared geometry/bounding box). I'm working to fix this issue (BRIN has infrastructure to deal with this case), and make sure the tests cover all cross dimension mixes.
I'll provide a new patch containing the 3 fixes and the new regression tests in a few days, so I can check and double check everything.
comment:4 by , 8 years ago
Owner: | changed from | to
---|
by , 8 years ago
Attachment: | brin_allfix_and_regtest-v1.diff added |
---|
comment:5 by , 8 years ago
I just attached brin_allfix_and_regtest-v1.diff (against svn-2.3), it's a single patch containing the fixes for the 2 issues mentionned before and the memory leak, add regression tests for all problematic tests, also fixes some typo in documentation and add some more information about the available operator classes and some advice about BRIN indexes with geometries of mixed dimensions.
All the regression tests works after this patch, I didn't try to build documentation though.
Tell me if you prefer to split in multiple patches, or if anything else is needed. Thanks!
comment:7 by , 8 years ago
Summary: | Bugs in BRIN support → Bugs in BRIN support - Index corruption and memory leak |
---|
Also, I'll also submit shortly additional regression tests which will cover these code path. I wanted to submit the bugfix for review as quickly as possible.