Manually delete physical files for ENR entries when leaving scope. #450
Conversation
Signed-off-by: Jason Teng <[email protected]>
Sairakan
changed the title
| /* | ||
| * Also register the physical file to be deleted at the end of scope. | ||
| */ | ||
| ENRDelete(enr->md.reliddesc); |
There was a problem hiding this comment.
Why not delete the file right here rather than adding it to delete list and later calling smgrDoPendingDeletes. We can write a new function to delete a file in SMGR
There was a problem hiding this comment.
Let me look into this. The original reasoning behind doing it this way is that I am not sure how much of the storage deletion needs to be copied, vs just immediately calling smgr_close() on the relation
There was a problem hiding this comment.
I've implemented this suggestion - now the files are immediately cleaned up during this function, which has the added benefit of not needing to modify any other function signatures or anything else in the code.
…es." This reverts commit f22b9e4.
Signed-off-by: Jason Teng <[email protected]>
Sairakan
changed the title
Signed-off-by: Jason Teng <[email protected]>
| srel = smgropen(rel->rd_locator, rel->rd_backend); | ||
|
|
||
| /* allocate the initial array, or extend it, if needed */ | ||
| if (maxrels == 0) |
There was a problem hiding this comment.
why are we doing this? we have single relationship, we should just pass pointer to that relation with count as 1 to smgrdounlinkall
There was a problem hiding this comment.
We do not necessarily only have one relation - there can be multiple table variables and temp tables defined in the same scope, which would all be cleaned up at once by this function.
There was a problem hiding this comment.
Correction: currently table variables are being cleaned up prior to this function during pltsql_clean_table_variables(), which manually executes a DROP TABLE statement to drop the table variable. Since that function only registers the physical file for deletion at transaction rather than immediately, we probably should remove that function and let this function do both table variable and temp table cleanup, but that can be done in a later commit.
TLDR: currently only temp tables in scope are being cleaned up here, and not table variables, which are handled separately.
lejaokri
merged commit f5394b4
into
babelfish-for-postgresql:BABEL_4_X_DEV__PG_16_X
…ope. (babelfish-for-postgresql#450) Add separate list to track pending deletes for TSQL ENR entries. Signed-off-by: Jason Teng <[email protected]>
…ope. (babelfish-for-postgresql#450) Add separate list to track pending deletes for TSQL ENR entries. Signed-off-by: Jason Teng <[email protected]>
Description
Second attempt at babelfish-for-postgresql/babelfish_extensions#2930 (which was reverted). This time, we completely ignore the pending deletes list, and instead manually trigger file deletion for each of the ENR entries we are dropping during ENRDropTempTables(). This ensures that we don't need to mess with the pendingDeletes list at all, while still properly deleting the physical storage for ENR entries when they are no longer in scope (such as at the end of a stored procedure call, explicit transaction, or batch).
Issues Resolved
BABEL-4751
Check List
By submitting this pull request, I confirm that my contribution is under the terms of the PostgreSQL license, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.