[pull] master from postgres:master #95
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
Adds a regression test with gin_index_check() on a multicolumn index, to verify it's handled correctly and improve test coverage for code introduced by 14ffaec. Author: Arseniy Mukhin <[email protected]> Reviewed-by: Andrey M. Borodin <[email protected]> Discussion: https://postgr.es/m/CAE7r3MJ611B9TE=YqBBncewp7-k64VWs+sjk7XF6fJUX77uFBA@mail.gmail.com
The field was introduced by commit 14ffaec, but is unused and unnecessary. So remove it. Issues reported by Arseniy Mukhin, along with a proposed patch. Review by Andrey M. Borodin, cleanup and minor improvements by me. Author: Arseniy Mukhin <[email protected]> Reviewed-by: Andrey M. Borodin <[email protected]> Discussion: https://postgr.es/m/CAE7r3MJ611B9TE=YqBBncewp7-k64VWs+sjk7XF6fJUX77uFBA@mail.gmail.com
This tightens a couple checks in checking GIN indexes, which might have resulted in incorrect results (false positives/negatives). * The code skipped ordering checks if the entries were for different attributes (for multi-column GIN indexes), possibly missing some cases of data corruption. But the attribute number is part of the ordering, so we can check that. * The root page was skipped when checking entry order, but that is unnecessary. The root page is subject to the same ordering rules, we can process it just like any other page. * The high key on the right-most page was not checked, but that is needed only for inner pages (we don't store the high key for those). For leaf pages we can check the high key just fine. * Correct the detection of split pages. If the page gets split, the cached parent key is greater than the current child key (not less, as the code incorrectly expected). Issues reported by Arseniy Mukhin, along with a proposed patch. Review by Andrey M. Borodin, cleanup and improvements by me. Author: Arseniy Mukhin <[email protected]> Reviewed-by: Andrey M. Borodin <[email protected]> Discussion: https://postgr.es/m/CAE7r3MJ611B9TE=YqBBncewp7-k64VWs+sjk7XF6fJUX77uFBA@mail.gmail.com
The checks introduced by commit 14ffaec did not get the parent key checks quite right, missing some data corruption cases. In particular: * The "rightlink" check was not working as intended, because rightlink is a BlockNumber, and InvalidBlockNumber is 0xFFFFFFFF, so !GinPageGetOpaque(page)->rightlink almost always evaluates to false (except for rightlink=0). So in most cases parenttup was left NULL, preventing any checks against parent. * Use GinGetDownlink() to retrieve child blkno to avoid triggering Assert, same as the core GIN code. Issues reported by Arseniy Mukhin, along with a proposed patch. Review by Andrey M. Borodin, cleanup and improvements by me. Author: Arseniy Mukhin <[email protected]> Reviewed-by: Andrey M. Borodin <[email protected]> Discussion: https://postgr.es/m/CAE7r3MJ611B9TE=YqBBncewp7-k64VWs+sjk7XF6fJUX77uFBA@mail.gmail.com
Fix two issues in parent_key validation in posting trees: * It's not enough to check stack->parentblk is valid to determine if the parentkey is valid. It's possible parentblk is set to a valid block number, but parentkey is invalid. So check parentkey directly. * We don't need to invalidate parentkey for all child pages of the rightmost page. It's enough to invalidate it for the rightmost child only, which means we can check more cases (less false negatives). Issues reported by Arseniy Mukhin, along with a proposed patch. Review by Andrey M. Borodin, cleanup and improvements by me. Author: Arseniy Mukhin <[email protected]> Reviewed-by: Andrey M. Borodin <[email protected]> Discussion: https://postgr.es/m/CAE7r3MJ611B9TE=YqBBncewp7-k64VWs+sjk7XF6fJUX77uFBA@mail.gmail.com
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.1)
Can you help keep this open source service alive? 💖 Please sponsor : )