Merge conflict in a61b1f74823c9c4f79c95226a461f1e7a367764b Rework query relation permission checking #85
Conversation
This file contains 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
Sairakan
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Dec 7, 2023
Signed-off-by: Jason Teng <[email protected]>
Sairakan
commented
Dec 7, 2023
| rte->requiredPerms = ACL_SELECT; | ||
| rte->insertedCols = NULL; | ||
| rte->updatedCols = NULL; | ||
| perminfo->requiredPerms = ACL_SELECT; |
There was a problem hiding this comment.
This is one section that needed to be manually modified in addition to the merge conflict
Sairakan
commented
Dec 7, 2023
| target_rte->updatedCols = | ||
| bms_add_member(target_rte->updatedCols, | ||
| target_perminfo->updatedCols = | ||
| bms_add_member(target_perminfo->updatedCols, |
There was a problem hiding this comment.
This is the other section that needed to be manually updated.
Sairakan
force-pushed
the
pg16-cherry-pick-bff-1
branch
from
f8f2011 to
8e03977
Compare
Sairakan
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Dec 7, 2023
Signed-off-by: Jason Teng <[email protected]>
BABELFISH-CONFLICT: see Postgres community repo for original commit Currently, information about the permissions to be checked on relations mentioned in a query is stored in their range table entries. So the executor must scan the entire range table looking for relations that need to have permissions checked. This can make the permission checking part of the executor initialization needlessly expensive when many inheritance children are present in the range range. While the permissions need not be checked on the individual child relations, the executor still must visit every range table entry to filter them out. This commit moves the permission checking information out of the range table entries into a new plan node called RTEPermissionInfo. Every top-level (inheritance "root") RTE_RELATION entry in the range table gets one and a list of those is maintained alongside the range table. This new list is initialized by the parser when initializing the range table. The rewriter can add more entries to it as rules/views are expanded. Finally, the planner combines the lists of the individual subqueries into one flat list that is passed to the executor for checking. To make it quick to find the RTEPermissionInfo entry belonging to a given relation, RangeTblEntry gets a new Index field 'perminfoindex' that stores the corresponding RTEPermissionInfo's index in the query's list of the latter. ExecutorCheckPerms_hook has gained another List * argument; the signature is now: typedef bool (*ExecutorCheckPerms_hook_type) (List *rangeTable, List *rtePermInfos, bool ereport_on_violation); The first argument is no longer used by any in-core uses of the hook, but we leave it in place because there may be other implementations that do. Implementations should likely scan the rtePermInfos list to determine which operations to allow or deny. Author: Amit Langote <[email protected]> Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com (cherry picked from commit a61b1f74823c9c4f79c95226a461f1e7a367764b)
Sairakan
force-pushed
the
pg16-cherry-pick-bff-1
branch
from
8e03977 to
9c0d528
Compare
Sairakan
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Dec 7, 2023
Signed-off-by: Jason Teng <[email protected]>
Must save-and-restore the context we are modifying. Oversight in commit a61b1f748. Tender Wang Discussion: https://postgr.es/m/CAHewXNnnNySD_YcKNuFpQDV2gxWA7_YLWqHmYVcyoOYxn8kY2A@mail.gmail.com Discussion: https://postgr.es/m/[email protected] (cherry picked from commit c7468c73f7b6e842a53c12eaee5578a76a8fa7a6)
Sairakan
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Dec 7, 2023
Signed-off-by: Jason Teng <[email protected]>
Sairakan
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Dec 7, 2023
Signed-off-by: Jason Teng <[email protected]>
2jungkook
approved these changes
Dec 7, 2023
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.
This commit moves relation permission checking out of the RangeTableEntry struct into its own struct, which causes conflicts in places where we have modified permission checking for relations.
Also need to cherry-pick in
c7468c73f7b6e842a53c12eaee5578a76a8fa7a6 Fix buggy recursion in flatten_rtes_walker().otherwise some Babelfish tests fail (this commit is ~500 commits down the line, so it is not practical to wait for it).Extension PR for validation: amazon-aurora/babelfish_extensions#54
Merge conflicts:
Merge conflict in src/backend/executor/execMain.c - resolved by taking both #include changes, and then only taking in the comment change
Merge conflict in src/backend/optimizer/plan/planner.c - resolved by taking both changes
Merge conflict in src/backend/parser/parse_relation.c - resolved by taking incoming changes
In addition,
addRangeTableEntryForENR()inparse_relation.candapply_handle_update()inworker.cneeded to be fixed, as they changed the code in a way that causes build errors but was not caught in the merge conflict.