implementation of table pragmas generator#212
Open
earthquakeonmars wants to merge 1 commit into
Open
Conversation
…ble_pragmas
The new function plpgsql_check_generate_table_pragmas(funcoid, relid, fatal_errors)
scans the body of a plpgsql function, and for every CREATE TEMP TABLE AS statement
there generates the table pragma string. The names and types of columns are derived
from planning of the inner query, so the pragmas don't need to be written (and
maintained) manually.
Generated pragmas can be passed to the new "pragmas" option (text array) of
plpgsql_check_function and plpgsql_check_function_tb. These pragmas are applied
before the start of static analysis:
create function fx()
returns void as $$
begin
create temp table xx as select a, b from tab;
insert into xx values (10, 'hello');
end;
$$ language plpgsql;
select * from plpgsql_check_generate_table_pragmas('fx()');
-- table: xx(a integer, b text)
-- now the check doesn't fail on missing temporary table
select * from plpgsql_check_function('fx()',
pragmas => array(select plpgsql_check_generate_table_pragmas('fx()')));
Only statically written CREATE TEMP TABLE AS SELECT|VALUES|TABLE commands are
processed, dynamic SQL is ignored. Found temporary tables are registered by the
table pragma mechanism immediately, so the following statements of the function's
body can use them. The check of the target schema (42P16) is done for all forms
of temporary tables. When the option fatal_errors is false, then failed statements
are skipped with a warning only. The generator has no side effects - all created
objects are removed by rollback of the check's subtransaction.
This is SQL API change - the extension version is bumped to 2.10.
Owner
|
it looks well - please change prefix from plpgsql_check_ to plch_ . I use it for new code. |
This file contains hidden or 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
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.
Table pragmas generator for CREATE TEMP TABLE ... AS statements
Motivation
When a PL/pgSQL function creates a temporary table with
CREATE TEMP TABLE ... AS SELECTand then references that table in the following statements, plpgsql_check reports
error:42P01 ... relation ... does not exist- the checker doesn't execute the body, sothe table is never created. The existing workaround - a manually written
plpgsql_check_pragma('table: t(col1 type1, ...)')- works, but the column list must beduplicated by hand and it gets out of sync easily when the query changes.
This PR makes these pragmas generated automatically.
What is new
Two additions to the SQL API:
a new function
It scans the function's body and returns one
table: name(col type, ...)pragma stringfor every
CREATE TEMP TABLE ... AS SELECT|VALUES|TABLEstatement there. Column namesand types are derived from planning of the inner query (the query is planned, never
executed).
relidhas the same meaning as inplpgsql_check_function- it is neededfor trigger functions.
a new option
pragmas text[] DEFAULT NULLfor all four variants ofplpgsql_check_functionandplpgsql_check_function_tb. Any pragma string is acceptedthere and applied before the check is started.
The whole workflow is one line:
Example:
The returned text can be edited by the user before it is passed to the
pragmasoption.The README got a new section "Table pragmas generator" (plus the
pragmasoption isdocumented in the arguments list, and the "Temporary tables" limits section points to the
generator now).
Versioning and compatibility
This is an SQL API change, so I bumped the extension version to 2.10 in all the places
(following the pattern of the 2.7 -> 2.8 bump): the script is renamed to
plpgsql_check--2.10.sql,default_versionin the control file,EXPECTED_EXTVERSION,PG_MODULE_MAGIC_EXT(2.10.0), meson project version, META.json,DATAin Makefile, andall seven spec files.
Please confirm the version number choice (2.10.0) - release numbering is yours, and I
don't want your "prepare for X.Y.Z" process to trip over an already-done bump. I can
renumber the whole set easily if you prefer something else.
Backward compatibility: the new parameters are appended at the end of the signatures
with
DEFAULT NULL, so all existing calls keep working unchanged. Since the projectdoesn't support
ALTER EXTENSION ... UPDATE, users have to drop and create the extensionafter upgrading the package - just a reminder, nothing new here.
Implementation notes (things worth a closer look in review)
plpgsql_check_function_internal(new flaggenerate_pragmasinplpgsql_check_info), so it reuses SPI, the fake fcinfo/estate,the plpgsql compilation and - most importantly - the check's always-rolled-back
subtransaction. Result: zero catalogue artifacts and idempotence (pinned by a
regress test that counts
pg_classrows before/after repeated calls).(
PERFORM plpgsql_check_pragma(...)and thePERFORM 'PRAGMA: ...'short form) ismoved from the static
ExprGetQueryinto the exportedplpgsql_check_apply_inline_pragmas, so the generator can reuse it. Please review thishunk primarily for equivalence with the old code.
plpgsql_check_expr_prepare_plan(a thin wrapper around thestatic
prepare_planwithout plan checks) andplpgsql_check_put_text_line(a wrapperaround the static
put_text_line).42P16check ("cannot create temporary relation in non-temporary schema") isdetected from the raw parse tree by the generator itself, intentionally not via
RangeVarAdjustRelationPersistence- that routine could create the session's temporarynamespace as a side effect.
fatal_errors => falsesemantics: a failed statement is skipped with a WARNING thatkeeps the original SQLSTATE and message, and the scanning continues. With the default
fatal_errors => truethe first failure is raised.result rows - the output stays a clean list of pragmas, safe to feed into the
pragmasoption programmatically.
Behaviour details
FROM- joins, subqueries, CTE and recursiveCTE queries work.
IF branches and exception handlers are scanned too); pragmas are returned positionally,
without deduplication by name.
mechanism), so chains "temp table built from an earlier temp table" resolve correctly.
PERFORM plpgsql_check_pragma('table: ...')calls in the body arerespected - a manually declared table is visible when the following CTAS statements are
planned.
character varying(10),numeric(12,2)), identifiersare quoted when necessary,
unknownis replaced bytext(likeCREATE TABLE ASdoes), explicit column lists
t(c1, c2) AS ...override the names positionally.What the function does not handle
1. Intentionally skipped, but still schema-checked (42P16)
All forms based on the
CreateStmtparse node:CREATE TEMP TABLE t (id int, name text);CREATE TEMP TABLE t (LIKE src INCLUDING ALL);CREATE TEMP TABLE t OF my_type;TEMPkeyword, via the schema -CREATE TABLE pg_temp.t (id int);(recognized as temporary, but no pragma is generated - it is the list form)
INHERITS (parent),PARTITION OF parent FOR VALUES ..., anyWITH (...)/USING method- all of these are the sameCreateStmtnode, thegenerator doesn't distinguish them.
For all of them the target schema is still validated:
CREATE TEMP TABLE public.t (...)raises
42P16(or a WARNING withfatal_errors => false), but no pragma string isreturned. Because these forms produce the
CreateStmtparse node (a different shape thanCreateTableAsStmt), the code for them will be done a bit later, as a follow-up.2. Not visible to the generator at all
EXECUTE 'CREATE TEMP TABLE ...', includingEXECUTE format(...)andquery text from a variable. The workaround is a manual
plpgsql_check_pragma('table: ...')next to theEXECUTE- the generator sees it andtakes it into account for the following CTAS statements.
CREATE TEMP TABLE t AS EXECUTE prep_stmt- CTAS over a prepared statement; thestructure cannot be derived statically, it is silently skipped.
PERFORM helper()/CALL proc()where the calleecreates the temporary table. Only the body of the checked function is scanned, the
generator doesn't descend into callees.
SELECT ... INTO TEMP t- in plain SQL this creates a table, but inside plpgsql thissyntax is not possible (
INTOis taken by plpgsql itself), and viaEXECUTEit isdynamic SQL again.
3. Pragma string is returned, but cannot be applied
Corner cases where the output is honest, but registering/using the pragma ends with the
existing pragma parser's WARNING:
CREATE TEMP TABLE t AS SELECT FROM src;returnstable: t(),but the pragma mechanism doesn't parse an empty column list.
SELECT a+1, a+1returnstable: t("?column?" integer, "?column?" integer); note that such CTAS would fail atruntime too (
column "?column?" specified more than once), so the function is brokenby itself.
4. Lost details (precision limits, not failures)
COLLATEis not carried into the pragma - thetable:pragma parser doesn'tsupport it, so a table registered by the pragma gets default collations.
CREATE TEMP SEQUENCE- thesequence:pragma exists in plpgsql_check, butthe generator produces only
table:pragmas; temporary sequences still have to bedeclared manually.
Testing and CI
plpgsql_check_pragma_generator(a single expected file for allsupported versions) covering: all CTAS forms (
AS SELECT/AS VALUES/AS TABLE,WITH NO DATA,IF NOT EXISTS,ON COMMITvariants, explicit column lists), name andtype rendering (quoting,
?column?,unknown->text, arrays/composites/domains/typmods, resjunk ORDER BY columns), body traversal (IF branches, loops, nested blocks,
exception handlers, unreachable code), positional duplicates, table-from-table chains,
shadowing of a real table,
pg_tempqualification, skipped forms, plpgsql variablesinside the query, overloaded functions and OUT parameters, trigger functions with
relid, error cases (42P01/42P16/too many column names,fatal_errors => falsebranches with pinned SQLSTATEs), idempotence/no-artifacts, and the
pragmasoptionend-to-end (incl. NULL/empty array, NULL elements, broken pragma string).
REGRESSin Makefile and to the meson tests list (side note:the meson list historically doesn't run the profiler test; the new test is included).
expected/READMEis updated.covered only by CI here - if the output drifts on some version, I'm ready to add
alternative expected files. The only version-sensitive assumption in the code is the
CreateTableAsStmt.objtypefield name (it is alreadyobjtypein PG14).Deferred work
CreateStmt-based forms of temporary tables (columndefinition list,
LIKE,OF type_name) - planned as a follow-up.