-
Notifications
You must be signed in to change notification settings - Fork 56
Add branch-aware temp table tracking during function analysis #198
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.
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
Conversation
Track temporary tables created via CREATE TEMP TABLE in EXECUTE statements so the linter doesn't report false "relation does not exist" errors. Uses scope-based tracking where temp tables created inside branches (IF, CASE, LOOP, etc.) are only visible within that branch and are cleaned up when the branch exits.
4df92d7 to
bf969e7
Compare
|
Hi parsing DDL statements and use this information is interesting idea that can possibly to reduce a necessity to use pragma I don't know about any rule that say so temporary table should be used only in current branch. So the main goal of this PR is not correct. Generally life cycle of any database objects is independent on structure of plpgsql statements. Temporary table exists until it is dropped. So this case is fully correct (and you should not to raise any error) More - creating and dropping temporary tables is slow (when it is very frequent), so very common pattern is just create temporary table in login trigger, and later use it without creating and dropping (use truncate instead drop). I believe so following pattern can be common: or Do you have some real problem, that you would to detect? Implementation is little bit broken and dirty (how you write) - you can see broken tests. Execution of |
|
I see. So I was thinking about that case and in the general case it's not possible as you know for sure (Rice's theorem). IMO this code arguably "should not pass". Just like a lot of correct code is often rejected by type checkers in other languages - they ask the user to reorganize the code and in return the user gets the benefit of having a reliable type system.
Yes, we had some functions that created temp tables inside of them and that's why I decided to look into it.
You are 100% correct. This PR was created with an assumption that it would introduce a slightly opinionated way to track this. I am wondering if you'd be open to this idea if we also add some sort of let me know and i'll eiether close the PR or work on it, this was just a conversation starter. |
|
čt 22. 1. 2026 v 8:46 odesílatel Wojtek Czekalski ***@***.***>
napsal:
*wokalski* left a comment (okbob/plpgsql_check#198)
<#198 (comment)>
I see. So I was thinking about that case
if v > 0 then
create temp table temp_either(id int);
else
create temp table temp_either(id int);
end if;
and in the general case it's not possible as you know for sure (Rice's
theorem). IMO this code arguably "should not pass". Just like a lot of
correct code is often rejected by type checkers in other languages - they
ask the user to reorganize the code and in return the user gets the benefit
of having a reliable type system.
I am sorry, but I don't see any significant problem in this case. Can you
describe it better, please?
Message ID: ***@***.***>
… |
|
In this particular case it's not a problem indeed and heuristics can be applied but in general if you have some wider tables it's not possible in the general case. For example: IF is_mammal THEN
CREATE TEMP TABLE feeding_schedule (
id SERIAL PRIMARY KEY,
food_type TEXT
);
END IF;
if x.race = 'human' then
perform * from feeding_schedule;
fiThe application developer knows that a runtime error is "impossible" (wink wink) here but the type checker cannot know this. It's same as in some languages you can make code like this that works: if x.is_mammal:
feeding_schedule = {
"id": 1,
"food_type": "Omnivore Diet"
}
if x.race == "human":
print(f"Human detected. Schedule: {feeding_schedule}")and in others it won't if (x.is_mammal) {
let feeding_schedule = {
id: 1
food_type: "Omnivore Diet",
}
}
if (x.race == "human") {
console.log("Human detected. Schedule: ", feeding_schedule);
}That said I do agree that there are some trivial cases like above that we could apply some heuristics for but I don't know if it's worth it because it can be simply refactored into: create temp table temp_either(id int);
if v > 0 then
-- do something
else
-- do something else
end if; |
|
Now, I better understand to your case, and I afraid so this cannot be implemented like you proposed. This really needs real list of branches and dependencies, and some partial solution based on forcing convention is worse than better. Few years ago there was similar request related to better work with polymorphic arguments (or maybe triggers) if I remember well. I don't think so this is possible in current design of plpgsql_check. It needs different implementation where branches are stored in better graph form with much deeper analyses of expressions. Internal structures of plpgsql_check are based on plpgsql structures, and I it is not optimized for analyses like this. More I afraid so it cannot work well, when the context of the analyse is limited only to one specific function. |
|
Understood, closing this then. Regardless thanks for the project it's really cool 😄. |
|
Thank you :-) |
Rough prototype to track temp tables created via
CREATE TEMP TABLEin EXECUTE statements, so the linter doesn't report false "relation does not exist" errors.Uses scope-based tracking - temp tables created inside branches (IF, CASE, LOOP) are only visible within that branch and cleaned up on exit. This means code that uses a temp table outside the branch where it was created will still error, which I think is correct behavior.
Compiled and tested on PostgreSQL 17. All regression tests pass.
Question: Do you agree with the scope-based approach? If yes, I'll clean this up properly.