-
Notifications
You must be signed in to change notification settings - Fork 2
Comparing changes
Open a pull request
base repository: postgresql-cfbot/postgresql
base: cf/5246~1
head repository: postgresql-cfbot/postgresql
compare: cf/5246
- 8 commits
- 24 files changed
- 2 contributors
Commits on Mar 31, 2025
-
Support cached plans that work from a parse-analyzed Query.
Up to now, plancache.c dealt only with raw parse trees as the starting point for a cached plan. However, we'd like to use this infrastructure for SQL functions, and in the case of a new-style SQL function we'll only have the stored querytree, which corresponds to an analyzed-but-not-rewritten Query. Fortunately, we can make plancache.c handle that scenario with only minor modifications; the biggest change is in RevalidateCachedQuery() where we will need to apply only pg_rewrite_query not pg_analyze_and_rewrite. This patch just installs the infrastructure; there's no caller as yet. Author: Alexander Pyhalov <[email protected]> Reviewed-by: Tom Lane <[email protected]> Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
Configuration menu - View commit details
-
Copy full SHA for c43855e - Browse repository at this point
Copy the full SHA c43855eView commit details -
Provide a post-rewrite callback hook in plancache.c.
SQL-language functions sometimes want to modify the targetlist of the query that returns their result. If they're to use the plan cache, it needs to be possible to do that over again when a replan occurs. Invent a callback hook to make that happen. I chose to provide a separate function SetPostRewriteHook to install such hooks. An alternative API could be to add two more arguments to CompleteCachedPlan. I didn't do so because I felt that few callers will want this, but there's a case that that way would be cleaner. Author: Tom Lane <[email protected]> Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
Configuration menu - View commit details
-
Copy full SHA for f8bf567 - Browse repository at this point
Copy the full SHA f8bf567View commit details -
Factor out plpgsql's management of its function cache.
SQL-language functions need precisely this same functionality to manage a long-lived cache of functions. Rather than duplicating or reinventing that code, let's split it out into a new module funccache.c so that it is available for any language that wants to use it. This is mostly an exercise in moving and renaming code, and should not change any behavior. I have added one feature that plpgsql doesn't use but SQL functions will need: the cache lookup key can include the output tuple descriptor when the function returns composite. Author: Tom Lane <[email protected]> Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
Configuration menu - View commit details
-
Copy full SHA for 5d8006c - Browse repository at this point
Copy the full SHA 5d8006cView commit details -
Restructure check_sql_fn_retval().
To support using the plan cache for SQL functions, we'll need to be able to redo the work of check_sql_fn_retval() on just one query's list-of-rewritten-queries at a time, since the plan cache will treat each query independently. This would be simple enough, except for a bizarre historical behavior: the existing code will take the last canSetTag query in the function as determining the result, even if it came from not-the-last original query. (The case is only possible when the last original query(s) are deleted by a DO INSTEAD NOTHING rule.) This behavior is undocumented except in source code comments, and it seems hard to believe that anyone's relying on it. It would be a mess to support with the plan cache, because a change in the rules applicable to some table could change which CachedPlanSource is supposed to produce the function result, even if the function itself has not changed. Let's just get rid of that silliness and insist that the last source query in the function is the one that must produce the result. Having mandated that, we can refactor check_sql_fn_retval() into an outer and an inner function where the inner one considers only a single list-of-rewritten-queries; the inner one will be usable in a post-rewrite callback hook as contemplated by the previous commit. Likewise refactor check_sql_fn_statements() so that we have a version that can be applied to just one list of Queries. (As things stand, it's not really necessary to recheck that during a replan, but maybe future changes in the rule system would create cases where it matters.) Also remove check_sql_fn_retval()'s targetlist output argument, putting the equivalent functionality into a separate function. This is needed because the plan cache would be in the way of passing that data directly. No outside caller needed that anyway. Author: Tom Lane <[email protected]> Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
Configuration menu - View commit details
-
Copy full SHA for 8c85086 - Browse repository at this point
Copy the full SHA 8c85086View commit details -
Add a test case showing undesirable RLS behavior in SQL functions.
In the historical implementation of SQL functions, once we have built a set of plans for a SQL function we'll continue to use them during subsequent function invocations in the same query. This isn't ideal, and this somewhat-contrived test case shows one reason why not: we don't notice changes in RLS-relevant state. I'm putting this as a separate patch in the series so that the change in behavior will be apparent. Author: Alexander Pyhalov <[email protected]> Reviewed-by: Tom Lane <[email protected]> Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
Configuration menu - View commit details
-
Copy full SHA for 81bc843 - Browse repository at this point
Copy the full SHA 81bc843View commit details -
Change SQL-language functions to use the plan cache.
In the historical implementation of SQL functions (when they don't get inlined), we built plans for the contained queries at first call within an outer query, and then re-used those plans for the duration of the outer query, and then forgot everything. This was not ideal, not least because the plans could not be customized to specific values of the function's parameters. Our plancache infrastructure seems mature enough to be used here. That will solve both the problem with not being able to build custom plans and the problem with not being able to share work across successive outer queries. Moreover, this reimplementation will react to events that should cause a replan at the next entry to the SQL function. This is illustrated in the change in the rowsecurity test, where we now detect an RLS context change that was previously ignored. (I also added a test in create_function_sql that exercises ShutdownSQLFunction(), after noting from coverage results that that wasn't getting reached.) Author: Alexander Pyhalov <[email protected]> Co-authored-by: Tom Lane <[email protected]> Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
Configuration menu - View commit details
-
Copy full SHA for 019a533 - Browse repository at this point
Copy the full SHA 019a533View commit details -
Delay parse analysis and rewrite until we're ready to execute the query.
This change fixes a longstanding bugaboo with SQL functions: you could not write DDL that would affect later statements in the same function. That's mostly still true with new-style SQL functions, since the results of parse analysis are baked into the stored query trees (and protected by dependency records). But for old-style SQL functions, it will now work much as it does with plpgsql functions. The key changes required are to (1) stash the parsetrees read from pg_proc somewhere safe until we're ready to process them, and (2) adjust the error context reporting. sql_compile_error_callback is now only useful for giving context for errors detected by raw parsing. Errors detected in either parse analysis or planning are handled by sql_exec_error_callback, as they were before this patch series. Author: Tom Lane <[email protected]> Discussion: https://postgr.es/m/8216639.NyiUUSuA9g@aivenlaptop
Configuration menu - View commit details
-
Copy full SHA for 05fabca - Browse repository at this point
Copy the full SHA 05fabcaView commit details -
[CF 5246] v11 - Allow SQL functions use CachedPlan machinery
This branch was automatically generated by a robot using patches from an email thread registered at: https://commitfest.postgresql.org/patch/5246 The branch will be overwritten each time a new patch version is posted to the thread, and also periodically to check for bitrot caused by changes on the master branch. Patch(es): https://www.postgresql.org/message-id/[email protected] Author(s): Alexander Pyhalov
Commitfest Bot committedMar 31, 2025 Configuration menu - View commit details
-
Copy full SHA for c6d38de - Browse repository at this point
Copy the full SHA c6d38deView commit details
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff cf/5246~1...cf/5246