Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manually delete physical files for ENR entries when leaving scope. #450

Merged
merged 6 commits into from
Sep 27, 2024
Merged

Manually delete physical files for ENR entries when leaving scope. #450

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f22b9e4
Add separate list to track pending deletes for TSQL ENR entries.
Sep 25, 2024
07883d2
Revert "Add separate list to track pending deletes for TSQL ENR entri…
Sep 26, 2024
cfe56aa
Manually delete physical files for ENR entries when leaving scope.
Sep 26, 2024
e766461
Clean up some code logic.
Sep 26, 2024
351db1c
Merge remote-tracking branch 'upstream/BABEL_4_X_DEV__PG_16_X' into j…
Sep 26, 2024
516a19b
Merge remote-tracking branch 'upstream/BABEL_4_X_DEV__PG_16_X' into j…
Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 48 additions & 3 deletions src/backend/utils/misc/queryenvironment.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "catalog/pg_shdepend.h"
#include "catalog/pg_index_d.h"
#include "parser/parser.h" /* only needed for GUC variables */
#include "storage/smgr.h"
#include "utils/inval.h"
#include "utils/guc.h"
#include "utils/syscache.h"
Expand Down Expand Up @@ -1549,9 +1550,12 @@ bool useTempOidBufferForOid(Oid relId)
void
ENRDropTempTables(QueryEnvironment *queryEnv)
{
ListCell *lc = NULL;
ObjectAddress object;
ObjectAddresses *objects;
ListCell *lc = NULL;
ObjectAddress object;
ObjectAddresses *objects;
int nrels = 0,
maxrels = 0;
SMgrRelation *srels = NULL;

if (!queryEnv)
return;
Expand All @@ -1564,6 +1568,8 @@ ENRDropTempTables(QueryEnvironment *queryEnv)
foreach(lc, queryEnv->namedRelList)
{
EphemeralNamedRelation enr = (EphemeralNamedRelation) lfirst(lc);
Relation rel;
SMgrRelation srel;

if (enr->md.enrtype != ENR_TSQL_TEMP)
continue;
Expand All @@ -1572,6 +1578,45 @@ ENRDropTempTables(QueryEnvironment *queryEnv)
object.objectSubId = 0;
object.objectId = enr->md.reliddesc;
add_exact_object_address(&object, objects);
/*
* Delete the physical storage for the relation.
* See: smgrDoPendingDeletes()
*/
rel = relation_open(enr->md.reliddesc, AccessExclusiveLock);
srel = smgropen(rel->rd_locator, rel->rd_backend);

/* allocate the initial array, or extend it, if needed */
if (maxrels == 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we doing this? we have single relationship, we should just pass pointer to that relation with count as 1 to smgrdounlinkall

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

{
maxrels = 8;
srels = palloc(sizeof(SMgrRelation) * maxrels);
}
else if (maxrels <= nrels)
{
maxrels *= 2;
srels = repalloc(srels, sizeof(SMgrRelation) * maxrels);
}

srels[nrels++] = srel;

relation_close(rel, NoLock);
}

/*
* The below call to performMultpleDeletions() only registers the
* underlying files for deletion at the end of transaction, but they
* need to be deleted now since we are exiting the current scope. We
* still need to run the function though, to make sure that all of the
* dependencies are properly cleaned up.
*/
if (nrels > 0)
{
smgrdounlinkall(srels, nrels, false);

for (int i = 0; i < nrels; i++)
smgrclose(srels[i]);

pfree(srels);
}

/*
Expand Down
Loading