Skip to content

Initial work over pg_amcheck for orioledb - #111

Open
bwyltcaxe wants to merge 1 commit into
patches18from
bt_index_check_oriole_implementation_core
Open

bwyltcaxe wants to merge 1 commit into
patches18from
bt_index_check_oriole_implementation_core

Conversation

@bwyltcaxe

Copy link
Copy Markdown

No description provided.

@za-arthur za-arthur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a discussion about amcheck implementation and IMHO we shouldn't verify orioledb tables/indexes inside heap or btree specific checks of amcheck.

@bwyltcaxe

Copy link
Copy Markdown
Author

We had a discussion about amcheck implementation and IMHO we shouldn't verify orioledb tables/indexes inside heap or btree specific checks of amcheck.

Hi!
As I see, the discussion is about tables verification. I agree that integrating the OrioleDB's table verification into verify_heapam is bad practice, but I don't think so for btree_index_check -- we are using btree and it uses kind of same invariants to check and this is the reason why I think it is the lesser of two evils.

I already tried to come up with the best solution: new function (as verify_orioledb_btree or smth like this). I've tried to look up at any other's extension precedents and I've not found any of them -- unfortunately, it looks like we're the pioneers here.

The main reason why I've decided to integrate directly into btree_index_check (except that we're using btree) is because I don't want to multiply boolean flags at pg_amcheck (which is hardcoded for heap).

I am ready to reimplement it using "flag + new function", but I am not sure this is what we should do

@bwyltcaxe

Copy link
Copy Markdown
Author

I think as we're the pioneers here, then another extension-developers will look at us too. If we will clone boolean flags, then they will too; hence, I think we have to come up with more-or-less common solutions, e.g. self-verification index hook as here. At btree_index_check or at smth like tableam_index_check.

Not sure about tables, so I didn't touch them here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants