Use RelationGetSmgr instead of rd_smgr #5028
Merged
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
Codecov Report
@@ Coverage Diff @@
## main #5028 +/- ##
==========================================
+ Coverage 89.58% 89.60% +0.01%
==========================================
Files 226 227 +1
Lines 51347 51584 +237
==========================================
+ Hits 46001 46222 +221
- Misses 5346 5362 +16
Continue to review full report at Codecov.
|
fabriziomello
approved these changes
Nov 27, 2022
Comment on lines
+706
to
+729
| #if PG13_GE | ||
| /* | ||
| * RelationGetSmgr | ||
| * Returns smgr file handle for a relation, opening it if needed. | ||
| * | ||
| * Very little code is authorized to touch rel->rd_smgr directly. Instead | ||
| * use this function to fetch its value. | ||
| * | ||
| * Note: since a relcache flush can cause the file handle to be closed again, | ||
| * it's unwise to hold onto the pointer returned by this function for any | ||
| * long period. Recommended practice is to just re-execute RelationGetSmgr | ||
| * each time you need to access the SMgrRelation. It's quite cheap in | ||
| * comparison to whatever an smgr function is going to do. | ||
| * | ||
| * copied verbatim from postgres because it is a static function | ||
| */ | ||
| static inline SMgrRelation | ||
| RelationGetSmgr(Relation rel) | ||
| { | ||
| if (unlikely(rel->rd_smgr == NULL)) | ||
| smgrsetowner(&(rel->rd_smgr), smgropen(rel->rd_node, rel->rd_backend)); | ||
| return rel->rd_smgr; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Shouldn't this code reside in the compat module?
There was a problem hiding this comment.
We use compat for wrapper functions that abstract API differences between the PG versions. For verbatim imports from PG source we use src/import but since copy.c was the only call site i put it there directly
rd_smgr should not be accessed directly but RelationGetSmgr should be used instead. Accessing it directly can lead to segfaults when parallel relcache flushes are happening. postgres/postgres@f10f0ae
konskov
approved these changes
Nov 28, 2022
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.
rd_smgr should not be accessed directly but RelationGetSmgr should be used instead. Accessing it directly can lead to segfaults when parallel relcache flushes are happening.
postgres/postgres@f10f0ae