Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1111b26

Browse filesBrowse files
tglsfdctglsfdc
committed
Undo decision to allow pg_proc.prosrc to be NULL.
Commit e717a9a changed the longstanding rule that prosrc is NOT NULL because when a SQL-language function is written in SQL-standard style, we don't currently have anything useful to put there. This seems a poor decision though, as it could easily have negative impacts on external PLs (opening them to crashes they didn't use to have, for instance). SQL-function-related code can just as easily test "is prosqlbody not null" as "is prosrc null", so there's no real gain there either. Hence, revert the NOT NULL marking removal and adjust related logic. For now, we just put an empty string into prosrc for SQL-standard functions. Maybe we'll have a better idea later, although the history of things like pg_attrdef.adsrc suggests that it's not easy to maintain a string equivalent of a node tree. This also adds an assertion that queryDesc->sourceText != NULL to standard_ExecutorStart. We'd been silently relying on that for awhile, so let's make it less silent. Also fix some overlooked documentation and test cases. Discussion: https://postgr.es/m/2197698.1617984583@sss.pgh.pa.us
1 parent 3157cbe commit 1111b26
Copy full SHA for 1111b26

File tree

Expand file treeCollapse file tree

15 files changed

+109
-79
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

15 files changed

+109
-79
lines changed
Open diff view settings
Collapse file

‎doc/src/sgml/catalogs.sgml‎

Copy file name to clipboardExpand all lines: doc/src/sgml/catalogs.sgml
+12-4Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6007,8 +6007,9 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
60076007
<structfield>prosqlbody</structfield> <type>pg_node_tree</type>
60086008
</para>
60096009
<para>
6010-
Pre-parsed SQL function body. This will be used for language SQL
6011-
functions if the body is not specified as a string constant.
6010+
Pre-parsed SQL function body. This is used for SQL-language
6011+
functions when the body is given in SQL-standard notation
6012+
rather than as a string literal. It's null in other cases.
60126013
</para></entry>
60136014
</row>
60146015

@@ -6036,9 +6037,16 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
60366037
<para>
60376038
For compiled functions, both built-in and dynamically loaded,
60386039
<structfield>prosrc</structfield> contains the function's C-language
6039-
name (link symbol). For all other currently-known language types,
6040+
name (link symbol).
6041+
For SQL-language functions, <structfield>prosrc</structfield> contains
6042+
the function's source text if that is specified as a string literal;
6043+
but if the function body is specified in SQL-standard style,
6044+
<structfield>prosrc</structfield> is unused (typically it's an empty
6045+
string) and <structfield>prosqlbody</structfield> contains the
6046+
pre-parsed definition.
6047+
For all other currently-known language types,
60406048
<structfield>prosrc</structfield> contains the function's source
6041-
text. <structfield>probin</structfield> is unused except for
6049+
text. <structfield>probin</structfield> is null except for
60426050
dynamically-loaded C functions, for which it gives the name of the
60436051
shared library file containing the function.
60446052
</para>
Collapse file

‎src/backend/catalog/pg_proc.c‎

Copy file name to clipboardExpand all lines: src/backend/catalog/pg_proc.c
+12-19Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ ProcedureCreate(const char *procedureName,
121121
/*
122122
* sanity checks
123123
*/
124-
Assert(PointerIsValid(prosrc) || PointerIsValid(prosqlbody));
124+
Assert(PointerIsValid(prosrc));
125125

126126
parameterCount = parameterTypes->dim1;
127127
if (parameterCount < 0 || parameterCount > FUNC_MAX_ARGS)
@@ -336,10 +336,7 @@ ProcedureCreate(const char *procedureName,
336336
values[Anum_pg_proc_protrftypes - 1] = trftypes;
337337
else
338338
nulls[Anum_pg_proc_protrftypes - 1] = true;
339-
if (prosrc)
340-
values[Anum_pg_proc_prosrc - 1] = CStringGetTextDatum(prosrc);
341-
else
342-
nulls[Anum_pg_proc_prosrc - 1] = true;
339+
values[Anum_pg_proc_prosrc - 1] = CStringGetTextDatum(prosrc);
343340
if (probin)
344341
values[Anum_pg_proc_probin - 1] = CStringGetTextDatum(probin);
345342
else
@@ -874,26 +871,29 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
874871
/* Postpone body checks if !check_function_bodies */
875872
if (check_function_bodies)
876873
{
874+
tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosrc, &isnull);
875+
if (isnull)
876+
elog(ERROR, "null prosrc");
877+
878+
prosrc = TextDatumGetCString(tmp);
879+
877880
/*
878881
* Setup error traceback support for ereport().
879882
*/
880883
callback_arg.proname = NameStr(proc->proname);
881-
callback_arg.prosrc = NULL;
884+
callback_arg.prosrc = prosrc;
882885

883886
sqlerrcontext.callback = sql_function_parse_error_callback;
884887
sqlerrcontext.arg = (void *) &callback_arg;
885888
sqlerrcontext.previous = error_context_stack;
886889
error_context_stack = &sqlerrcontext;
887890

888-
tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosrc, &isnull);
889-
if (isnull)
891+
/* If we have prosqlbody, pay attention to that not prosrc */
892+
tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosqlbody, &isnull);
893+
if (!isnull)
890894
{
891895
Node *n;
892896

893-
tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosqlbody, &isnull);
894-
if (isnull)
895-
elog(ERROR, "null prosrc and prosqlbody");
896-
897897
n = stringToNode(TextDatumGetCString(tmp));
898898
if (IsA(n, List))
899899
querytree_list = castNode(List, n);
@@ -902,10 +902,6 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
902902
}
903903
else
904904
{
905-
prosrc = TextDatumGetCString(tmp);
906-
907-
callback_arg.prosrc = prosrc;
908-
909905
/*
910906
* We can't do full prechecking of the function definition if there
911907
* are any polymorphic input types, because actual datatypes of
@@ -1001,9 +997,6 @@ function_parse_error_transpose(const char *prosrc)
1001997
int newerrposition;
1002998
const char *queryText;
1003999

1004-
if (!prosrc)
1005-
return false;
1006-
10071000
/*
10081001
* Nothing to do unless we are dealing with a syntax error that has a
10091002
* cursor position.
Collapse file

‎src/backend/commands/functioncmds.c‎

Copy file name to clipboardExpand all lines: src/backend/commands/functioncmds.c
+10-1Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,8 +958,17 @@ interpret_AS_clause(Oid languageOid, const char *languageName,
958958
*sql_body_out = (Node *) q;
959959
}
960960

961+
/*
962+
* We must put something in prosrc. For the moment, just record an
963+
* empty string. It might be useful to store the original text of the
964+
* CREATE FUNCTION statement --- but to make actual use of that in
965+
* error reports, we'd also have to adjust readfuncs.c to not throw
966+
* away node location fields when reading prosqlbody.
967+
*/
968+
*prosrc_str_p = pstrdup("");
969+
970+
/* But we definitely don't need probin. */
961971
*probin_str_p = NULL;
962-
*prosrc_str_p = NULL;
963972
}
964973
else
965974
{
Collapse file

‎src/backend/executor/execMain.c‎

Copy file name to clipboardExpand all lines: src/backend/executor/execMain.c
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
195195
palloc0(nParamExec * sizeof(ParamExecData));
196196
}
197197

198+
/* We now require all callers to provide sourceText */
199+
Assert(queryDesc->sourceText != NULL);
198200
estate->es_sourceText = queryDesc->sourceText;
199201

200202
/*
Collapse file

‎src/backend/executor/functions.c‎

Copy file name to clipboardExpand all lines: src/backend/executor/functions.c
+10-10Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,15 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
667667
procedureTuple,
668668
Anum_pg_proc_prosrc,
669669
&isNull);
670+
if (isNull)
671+
elog(ERROR, "null prosrc for function %u", foid);
672+
fcache->src = TextDatumGetCString(tmp);
673+
674+
/* If we have prosqlbody, pay attention to that not prosrc. */
675+
tmp = SysCacheGetAttr(PROCOID,
676+
procedureTuple,
677+
Anum_pg_proc_prosqlbody,
678+
&isNull);
670679

671680
/*
672681
* Parse and rewrite the queries in the function text. Use sublists to
@@ -678,18 +687,11 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
678687
* plancache.c.
679688
*/
680689
queryTree_list = NIL;
681-
if (isNull)
690+
if (!isNull)
682691
{
683692
Node *n;
684693
List *stored_query_list;
685694

686-
tmp = SysCacheGetAttr(PROCOID,
687-
procedureTuple,
688-
Anum_pg_proc_prosqlbody,
689-
&isNull);
690-
if (isNull)
691-
elog(ERROR, "null prosrc and prosqlbody for function %u", foid);
692-
693695
n = stringToNode(TextDatumGetCString(tmp));
694696
if (IsA(n, List))
695697
stored_query_list = linitial_node(List, castNode(List, n));
@@ -710,8 +712,6 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
710712
{
711713
List *raw_parsetree_list;
712714

713-
fcache->src = TextDatumGetCString(tmp);
714-
715715
raw_parsetree_list = pg_parse_query(fcache->src);
716716

717717
foreach(lc, raw_parsetree_list)
Collapse file

‎src/backend/optimizer/util/clauses.c‎

Copy file name to clipboardExpand all lines: src/backend/optimizer/util/clauses.c
+32-34Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4317,32 +4317,37 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
43174317
ALLOCSET_DEFAULT_SIZES);
43184318
oldcxt = MemoryContextSwitchTo(mycxt);
43194319

4320+
/* Fetch the function body */
4321+
tmp = SysCacheGetAttr(PROCOID,
4322+
func_tuple,
4323+
Anum_pg_proc_prosrc,
4324+
&isNull);
4325+
if (isNull)
4326+
elog(ERROR, "null prosrc for function %u", funcid);
4327+
src = TextDatumGetCString(tmp);
4328+
43204329
/*
43214330
* Setup error traceback support for ereport(). This is so that we can
43224331
* finger the function that bad information came from.
43234332
*/
43244333
callback_arg.proname = NameStr(funcform->proname);
4325-
callback_arg.prosrc = NULL;
4334+
callback_arg.prosrc = src;
43264335

43274336
sqlerrcontext.callback = sql_inline_error_callback;
43284337
sqlerrcontext.arg = (void *) &callback_arg;
43294338
sqlerrcontext.previous = error_context_stack;
43304339
error_context_stack = &sqlerrcontext;
43314340

4332-
/* Fetch the function body */
4341+
/* If we have prosqlbody, pay attention to that not prosrc */
43334342
tmp = SysCacheGetAttr(PROCOID,
43344343
func_tuple,
4335-
Anum_pg_proc_prosrc,
4344+
Anum_pg_proc_prosqlbody,
43364345
&isNull);
4337-
if (isNull)
4346+
if (!isNull)
43384347
{
43394348
Node *n;
43404349
List *querytree_list;
43414350

4342-
tmp = SysCacheGetAttr(PROCOID, func_tuple, Anum_pg_proc_prosqlbody, &isNull);
4343-
if (isNull)
4344-
elog(ERROR, "null prosrc and prosqlbody for function %u", funcid);
4345-
43464351
n = stringToNode(TextDatumGetCString(tmp));
43474352
if (IsA(n, List))
43484353
querytree_list = linitial_node(List, castNode(List, n));
@@ -4354,10 +4359,6 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
43544359
}
43554360
else
43564361
{
4357-
src = TextDatumGetCString(tmp);
4358-
4359-
callback_arg.prosrc = src;
4360-
43614362
/*
43624363
* Set up to handle parameters while parsing the function body. We need a
43634364
* dummy FuncExpr node containing the already-simplified arguments to pass
@@ -4658,15 +4659,12 @@ sql_inline_error_callback(void *arg)
46584659
int syntaxerrposition;
46594660

46604661
/* If it's a syntax error, convert to internal syntax error report */
4661-
if (callback_arg->prosrc)
4662+
syntaxerrposition = geterrposition();
4663+
if (syntaxerrposition > 0)
46624664
{
4663-
syntaxerrposition = geterrposition();
4664-
if (syntaxerrposition > 0)
4665-
{
4666-
errposition(0);
4667-
internalerrposition(syntaxerrposition);
4668-
internalerrquery(callback_arg->prosrc);
4669-
}
4665+
errposition(0);
4666+
internalerrposition(syntaxerrposition);
4667+
internalerrquery(callback_arg->prosrc);
46704668
}
46714669

46724670
errcontext("SQL function \"%s\" during inlining", callback_arg->proname);
@@ -4778,6 +4776,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
47784776
Oid func_oid;
47794777
HeapTuple func_tuple;
47804778
Form_pg_proc funcform;
4779+
char *src;
47814780
Datum tmp;
47824781
bool isNull;
47834782
MemoryContext oldcxt;
@@ -4886,31 +4885,36 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
48864885
ALLOCSET_DEFAULT_SIZES);
48874886
oldcxt = MemoryContextSwitchTo(mycxt);
48884887

4888+
/* Fetch the function body */
4889+
tmp = SysCacheGetAttr(PROCOID,
4890+
func_tuple,
4891+
Anum_pg_proc_prosrc,
4892+
&isNull);
4893+
if (isNull)
4894+
elog(ERROR, "null prosrc for function %u", func_oid);
4895+
src = TextDatumGetCString(tmp);
4896+
48894897
/*
48904898
* Setup error traceback support for ereport(). This is so that we can
48914899
* finger the function that bad information came from.
48924900
*/
48934901
callback_arg.proname = NameStr(funcform->proname);
4894-
callback_arg.prosrc = NULL;
4902+
callback_arg.prosrc = src;
48954903

48964904
sqlerrcontext.callback = sql_inline_error_callback;
48974905
sqlerrcontext.arg = (void *) &callback_arg;
48984906
sqlerrcontext.previous = error_context_stack;
48994907
error_context_stack = &sqlerrcontext;
49004908

4901-
/* Fetch the function body */
4909+
/* If we have prosqlbody, pay attention to that not prosrc */
49024910
tmp = SysCacheGetAttr(PROCOID,
49034911
func_tuple,
4904-
Anum_pg_proc_prosrc,
4912+
Anum_pg_proc_prosqlbody,
49054913
&isNull);
4906-
if (isNull)
4914+
if (!isNull)
49074915
{
49084916
Node *n;
49094917

4910-
tmp = SysCacheGetAttr(PROCOID, func_tuple, Anum_pg_proc_prosqlbody, &isNull);
4911-
if (isNull)
4912-
elog(ERROR, "null prosrc and prosqlbody for function %u", func_oid);
4913-
49144918
n = stringToNode(TextDatumGetCString(tmp));
49154919
if (IsA(n, List))
49164920
querytree_list = linitial_node(List, castNode(List, n));
@@ -4927,12 +4931,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
49274931
}
49284932
else
49294933
{
4930-
char *src;
4931-
4932-
src = TextDatumGetCString(tmp);
4933-
4934-
callback_arg.prosrc = src;
4935-
49364934
/*
49374935
* Set up to handle parameters while parsing the function body. We can
49384936
* use the FuncExpr just created as the input for
Collapse file

‎src/bin/pg_dump/pg_dump.c‎

Copy file name to clipboardExpand all lines: src/bin/pg_dump/pg_dump.c
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12247,7 +12247,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
1224712247

1224812248
if (fout->remoteVersion >= 140000)
1224912249
appendPQExpBufferStr(query,
12250-
"CASE WHEN prosrc IS NULL AND lanname = 'sql' THEN pg_get_function_sqlbody(p.oid) END AS prosqlbody\n");
12250+
"pg_get_function_sqlbody(p.oid) AS prosqlbody\n");
1225112251
else
1225212252
appendPQExpBufferStr(query,
1225312253
"NULL AS prosqlbody\n");
Collapse file

‎src/bin/psql/describe.c‎

Copy file name to clipboardExpand all lines: src/bin/psql/describe.c
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ describeFunctions(const char *functypes, const char *func_pattern,
512512
gettext_noop("Language"));
513513
if (pset.sversion >= 140000)
514514
appendPQExpBuffer(&buf,
515-
",\n COALESCE(p.prosrc, pg_catalog.pg_get_function_sqlbody(p.oid)) as \"%s\"",
515+
",\n COALESCE(pg_catalog.pg_get_function_sqlbody(p.oid), p.prosrc) as \"%s\"",
516516
gettext_noop("Source code"));
517517
else
518518
appendPQExpBuffer(&buf,
Collapse file

‎src/include/catalog/catversion.h‎

Copy file name to clipboardExpand all lines: src/include/catalog/catversion.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/* yyyymmddN */
56-
#define CATALOG_VERSION_NO 202104081
56+
#define CATALOG_VERSION_NO 202104151
5757

5858
#endif
Collapse file

‎src/include/catalog/pg_proc.h‎

Copy file name to clipboardExpand all lines: src/include/catalog/pg_proc.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ CATALOG(pg_proc,1255,ProcedureRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(81,Proce
112112
Oid protrftypes[1] BKI_DEFAULT(_null_) BKI_LOOKUP(pg_type);
113113

114114
/* procedure source text */
115-
text prosrc;
115+
text prosrc BKI_FORCE_NOT_NULL;
116116

117117
/* secondary procedure info (can be NULL) */
118118
text probin BKI_DEFAULT(_null_);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.