Thread: CONSTANT/NOT NULL/initializer properties for plpgsql record variables
I said a couple of times in recent threads that it wouldn't be too hard to implement $SUBJECT given the other patches I've been working on. Attached is proof of the pudding. This needs to be applied on top of the patches in https://postgr.es/m/23537.1514589378@sss.pgh.pa.us and https://postgr.es/m/11986.1514407114@sss.pgh.pa.us It's pretty straightforward really. Worth noting is that this also fixes the null-domain-value issues I mentioned as being a loose end in the first of the above-referenced messages. Also, I created a new plpgsql test file for these features, and moved the one relevant existing test case into that file. I was a bit disappointed to find that no documentation changes seem needed, because the SGML docs fail to acknowledge that these cases didn't work ... regards, tom lane diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile index f9a3616..ad93e3e 100644 *** a/src/pl/plpgsql/src/Makefile --- b/src/pl/plpgsql/src/Makefile *************** DATA = plpgsql.control plpgsql--1.0.sql *** 26,32 **** REGRESS_OPTS = --dbname=$(PL_TESTDB) ! REGRESS = plpgsql_call plpgsql_record all: all-lib --- 26,32 ---- REGRESS_OPTS = --dbname=$(PL_TESTDB) ! REGRESS = plpgsql_call plpgsql_record plpgsql_varprops all: all-lib diff --git a/src/pl/plpgsql/src/expected/plpgsql_varprops.out b/src/pl/plpgsql/src/expected/plpgsql_varprops.out index ...109056c . *** a/src/pl/plpgsql/src/expected/plpgsql_varprops.out --- b/src/pl/plpgsql/src/expected/plpgsql_varprops.out *************** *** 0 **** --- 1,300 ---- + -- + -- Tests for PL/pgSQL variable properties: CONSTANT, NOT NULL, initializers + -- + create type var_record as (f1 int4, f2 int4); + create domain int_nn as int not null; + create domain var_record_nn as var_record not null; + create domain var_record_colnn as var_record check((value).f2 is not null); + -- CONSTANT + do $$ + declare x constant int := 42; + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = 42 + do $$ + declare x constant int; + begin + x := 42; -- fail + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: x := 42; -- fail + ^ + do $$ + declare x constant int; y int; + begin + for x, y in select 1, 2 loop -- fail + end loop; + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: for x, y in select 1, 2 loop -- fail + ^ + do $$ + declare x constant int[]; + begin + x[1] := 42; -- fail + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: x[1] := 42; -- fail + ^ + do $$ + declare x constant int[]; y int; + begin + for x[1], y in select 1, 2 loop -- fail (currently, unsupported syntax) + end loop; + end$$; + ERROR: syntax error at or near "[" + LINE 4: for x[1], y in select 1, 2 loop -- fail (currently, unsup... + ^ + do $$ + declare x constant var_record; + begin + x.f1 := 42; -- fail + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: x.f1 := 42; -- fail + ^ + do $$ + declare x constant var_record; y int; + begin + for x.f1, y in select 1, 2 loop -- fail + end loop; + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: for x.f1, y in select 1, 2 loop -- fail + ^ + -- initializer expressions + do $$ + declare x int := sin(0); + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = 0 + do $$ + declare x int := 1/0; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: division by zero + CONTEXT: SQL statement "SELECT 1/0" + PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x bigint[] := array[1,3,5]; + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = {1,3,5} + do $$ + declare x record := row(1,2,3); + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = (1,2,3) + do $$ + declare x var_record := row(1,2); + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = (1,2) + -- NOT NULL + do $$ + declare x int not null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: variable "x" must have a default value, since it's declared NOT NULL + LINE 2: declare x int not null; -- fail + ^ + do $$ + declare x int not null := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = 42 + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment + do $$ + declare x int not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: variable "x" must have a default value, since it's declared NOT NULL + LINE 2: declare x record not null; -- fail + ^ + do $$ + declare x record not null := row(42); + begin + raise notice 'x = %', x; + x := row(null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = (42) + NOTICE: x = () + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 7 at assignment + do $$ + declare x record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: variable "x" must have a default value, since it's declared NOT NULL + LINE 2: declare x var_record not null; -- fail + ^ + do $$ + declare x var_record not null := row(41,42); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = (41,42) + NOTICE: x = (,) + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 7 at assignment + do $$ + declare x var_record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + -- Check that variables are reinitialized on block re-entry. + \set VERBOSITY terse \\ -- needed for output stability + do $$ + begin + for i in 1..3 loop + declare + x int; + y int := i; + r record; + c var_record; + begin + if i = 1 then + x := 42; + r := row(i, i+1); + c := row(i, i+1); + end if; + raise notice 'x = %', x; + raise notice 'y = %', y; + raise notice 'r = %', r; + raise notice 'c = %', c; + end; + end loop; + end$$; + NOTICE: x = 42 + NOTICE: y = 1 + NOTICE: r = (1,2) + NOTICE: c = (1,2) + NOTICE: x = <NULL> + NOTICE: y = 2 + NOTICE: r = <NULL> + NOTICE: c = <NULL> + NOTICE: x = <NULL> + NOTICE: y = 3 + NOTICE: r = <NULL> + NOTICE: c = <NULL> + \set VERBOSITY default + -- Check enforcement of domain constraints during initialization + do $$ + declare x int_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain int_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x int_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain int_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x int_nn := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = 42 + ERROR: domain int_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment + do $$ + declare x var_record_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain var_record_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain var_record_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_nn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + x := null; -- fail + end$$; + NOTICE: x = (1,2) + ERROR: domain var_record_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 6 at assignment + do $$ + declare x var_record_colnn; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_colnn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_colnn := row(1,null); -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = (1,2) + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- fail + end$$; + NOTICE: x = (1,2) + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 5c2e463..576924f 100644 *** a/src/pl/plpgsql/src/pl_comp.c --- b/src/pl/plpgsql/src/pl_comp.c *************** do_compile(FunctionCallInfo fcinfo, *** 594,604 **** errhint("The arguments of the trigger can be accessed through TG_NARGS and TG_ARGV instead."))); /* Add the record for referencing NEW ROW */ ! rec = plpgsql_build_record("new", 0, RECORDOID, true); function->new_varno = rec->dno; /* Add the record for referencing OLD ROW */ ! rec = plpgsql_build_record("old", 0, RECORDOID, true); function->old_varno = rec->dno; /* Add the variable tg_name */ --- 594,604 ---- errhint("The arguments of the trigger can be accessed through TG_NARGS and TG_ARGV instead."))); /* Add the record for referencing NEW ROW */ ! rec = plpgsql_build_record("new", 0, NULL, RECORDOID, true); function->new_varno = rec->dno; /* Add the record for referencing OLD ROW */ ! rec = plpgsql_build_record("old", 0, NULL, RECORDOID, true); function->old_varno = rec->dno; /* Add the variable tg_name */ *************** plpgsql_build_variable(const char *refna *** 1811,1817 **** var->refname = pstrdup(refname); var->lineno = lineno; var->datatype = dtype; ! /* other fields might be filled by caller */ /* preset to NULL */ var->value = 0; --- 1811,1817 ---- var->refname = pstrdup(refname); var->lineno = lineno; var->datatype = dtype; ! /* other fields are left as 0, might be changed by caller */ /* preset to NULL */ var->value = 0; *************** plpgsql_build_variable(const char *refna *** 1831,1837 **** /* Composite type -- build a record variable */ PLpgSQL_rec *rec; ! rec = plpgsql_build_record(refname, lineno, dtype->typoid, add2namespace); result = (PLpgSQL_variable *) rec; break; --- 1831,1838 ---- /* Composite type -- build a record variable */ PLpgSQL_rec *rec; ! rec = plpgsql_build_record(refname, lineno, ! dtype, dtype->typoid, add2namespace); result = (PLpgSQL_variable *) rec; break; *************** plpgsql_build_variable(const char *refna *** 1856,1862 **** * Build empty named record variable, and optionally add it to namespace */ PLpgSQL_rec * ! plpgsql_build_record(const char *refname, int lineno, Oid rectypeid, bool add2namespace) { PLpgSQL_rec *rec; --- 1857,1864 ---- * Build empty named record variable, and optionally add it to namespace */ PLpgSQL_rec * ! plpgsql_build_record(const char *refname, int lineno, ! PLpgSQL_type *dtype, Oid rectypeid, bool add2namespace) { PLpgSQL_rec *rec; *************** plpgsql_build_record(const char *refname *** 1865,1870 **** --- 1867,1874 ---- rec->dtype = PLPGSQL_DTYPE_REC; rec->refname = pstrdup(refname); rec->lineno = lineno; + /* other fields are left as 0, might be changed by caller */ + rec->datatype = dtype; rec->rectypeid = rectypeid; rec->firstfield = -1; rec->erh = NULL; *************** build_row_from_vars(PLpgSQL_variable **v *** 1899,1904 **** --- 1903,1911 ---- int32 typmod; Oid typcoll; + /* Member vars of a row should never be const */ + Assert(!var->isconst); + switch (var->dtype) { case PLPGSQL_DTYPE_VAR: diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index c9fbd83..e97be40 100644 *** a/src/pl/plpgsql/src/pl_exec.c --- b/src/pl/plpgsql/src/pl_exec.c *************** plpgsql_exec_function(PLpgSQL_function * *** 461,467 **** } else { ! /* If arg is null, treat it as an empty row */ exec_move_row(&estate, (PLpgSQL_variable *) rec, NULL, NULL); } --- 461,467 ---- } else { ! /* If arg is null, set variable to null */ exec_move_row(&estate, (PLpgSQL_variable *) rec, NULL, NULL); } *************** exec_stmt_block(PLpgSQL_execstate *estat *** 1461,1471 **** { /* * If needed, give the datatype a chance to reject ! * NULLs, by assigning a NULL to the variable. We * claim the value is of type UNKNOWN, not the var's ! * datatype, else coercion will be skipped. (Do this ! * before the notnull check to be consistent with ! * exec_assign_value.) */ if (var->datatype->typtype == TYPTYPE_DOMAIN) exec_assign_value(estate, --- 1461,1469 ---- { /* * If needed, give the datatype a chance to reject ! * NULLs, by assigning a NULL to the variable. We * claim the value is of type UNKNOWN, not the var's ! * datatype, else coercion will be skipped. */ if (var->datatype->typtype == TYPTYPE_DOMAIN) exec_assign_value(estate, *************** exec_stmt_block(PLpgSQL_execstate *estat *** 1475,1485 **** UNKNOWNOID, -1); ! if (var->notnull) ! ereport(ERROR, ! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), ! errmsg("variable \"%s\" declared NOT NULL cannot default to NULL", ! var->refname))); } else { --- 1473,1480 ---- UNKNOWNOID, -1); ! /* parser should have rejected NOT NULL */ ! Assert(!var->notnull); } else { *************** exec_stmt_block(PLpgSQL_execstate *estat *** 1493,1501 **** { PLpgSQL_rec *rec = (PLpgSQL_rec *) datum; ! if (rec->erh) ! DeleteExpandedObject(ExpandedRecordGetDatum(rec->erh)); ! rec->erh = NULL; } break; --- 1488,1515 ---- { PLpgSQL_rec *rec = (PLpgSQL_rec *) datum; ! /* ! * Deletion of any existing object will be handled during ! * the assignments below, and in some cases it's more ! * efficient for us not to get rid of it beforehand. ! */ ! if (rec->default_val == NULL) ! { ! /* ! * If needed, give the datatype a chance to reject ! * NULLs, by assigning a NULL to the variable. ! */ ! exec_move_row(estate, (PLpgSQL_variable *) rec, ! NULL, NULL); ! ! /* parser should have rejected NOT NULL */ ! Assert(!rec->notnull); ! } ! else ! { ! exec_assign_expr(estate, (PLpgSQL_datum *) rec, ! rec->default_val); ! } } break; *************** exec_assign_value(PLpgSQL_execstate *est *** 4739,4745 **** if (isNull) { ! /* If source is null, just assign nulls to the record */ exec_move_row(estate, (PLpgSQL_variable *) rec, NULL, NULL); } --- 4753,4765 ---- if (isNull) { ! if (rec->notnull) ! ereport(ERROR, ! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), ! errmsg("null value cannot be assigned to variable \"%s\" declared NOT NULL", ! rec->refname))); ! ! /* Set variable to a simple NULL */ exec_move_row(estate, (PLpgSQL_variable *) rec, NULL, NULL); } *************** exec_move_row(PLpgSQL_execstate *estate, *** 6442,6450 **** */ if (tupdesc == NULL) { ! if (rec->erh) ! DeleteExpandedObject(ExpandedRecordGetDatum(rec->erh)); ! rec->erh = NULL; return; } --- 6462,6488 ---- */ if (tupdesc == NULL) { ! if (rec->datatype && ! rec->datatype->typtype == TYPTYPE_DOMAIN) ! { ! /* ! * If it's a composite domain, NULL might not be a legal ! * value, so we instead need to make an empty expanded record ! * and ensure that domain type checking gets done. If there ! * is already an expanded record, piggyback on its lookups. ! */ ! newerh = make_expanded_record_for_rec(estate, rec, ! NULL, rec->erh); ! expanded_record_set_tuple(newerh, NULL, false); ! assign_record_var(estate, rec, newerh); ! } ! else ! { ! /* Just clear it to NULL */ ! if (rec->erh) ! DeleteExpandedObject(ExpandedRecordGetDatum(rec->erh)); ! rec->erh = NULL; ! } return; } diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c index f499d96..a2634c4 100644 *** a/src/pl/plpgsql/src/pl_funcs.c --- b/src/pl/plpgsql/src/pl_funcs.c *************** plpgsql_free_function_memory(PLpgSQL_fun *** 718,723 **** --- 718,728 ---- case PLPGSQL_DTYPE_ROW: break; case PLPGSQL_DTYPE_REC: + { + PLpgSQL_rec *rec = (PLpgSQL_rec *) d; + + free_expr(rec->default_val); + } break; case PLPGSQL_DTYPE_RECFIELD: break; *************** plpgsql_dumptree(PLpgSQL_function *func) *** 1589,1594 **** --- 1594,1609 ---- printf("REC %-16s typoid %u\n", ((PLpgSQL_rec *) d)->refname, ((PLpgSQL_rec *) d)->rectypeid); + if (((PLpgSQL_rec *) d)->isconst) + printf(" CONSTANT\n"); + if (((PLpgSQL_rec *) d)->notnull) + printf(" NOT NULL\n"); + if (((PLpgSQL_rec *) d)->default_val != NULL) + { + printf(" DEFAULT "); + dump_expr(((PLpgSQL_rec *) d)->default_val); + printf("\n"); + } break; case PLPGSQL_DTYPE_RECFIELD: printf("RECFIELD %-16s of REC %d\n", diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index df88ba3..e37f548 100644 *** a/src/pl/plpgsql/src/pl_gram.y --- b/src/pl/plpgsql/src/pl_gram.y *************** decl_statement : decl_varname decl_const *** 502,538 **** var = plpgsql_build_variable($1.name, $1.lineno, $3, true); ! if ($2) ! { ! if (var->dtype == PLPGSQL_DTYPE_VAR) ! ((PLpgSQL_var *) var)->isconst = $2; ! else ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("record variable cannot be CONSTANT"), ! parser_errposition(@2))); ! } ! if ($5) ! { ! if (var->dtype == PLPGSQL_DTYPE_VAR) ! ((PLpgSQL_var *) var)->notnull = $5; ! else ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("record variable cannot be NOT NULL"), ! parser_errposition(@4))); ! } ! if ($6 != NULL) ! { ! if (var->dtype == PLPGSQL_DTYPE_VAR) ! ((PLpgSQL_var *) var)->default_val = $6; ! else ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("default value for record variable is not supported"), ! parser_errposition(@5))); ! } } | decl_varname K_ALIAS K_FOR decl_aliasitem ';' { --- 502,521 ---- var = plpgsql_build_variable($1.name, $1.lineno, $3, true); ! var->isconst = $2; ! var->notnull = $5; ! var->default_val = $6; ! /* ! * The combination of NOT NULL without an initializer ! * can't work, so let's reject it at compile time. ! */ ! if (var->notnull && var->default_val == NULL) ! ereport(ERROR, ! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), ! errmsg("variable \"%s\" must have a default value, since it's declared NOT NULL", ! var->refname), ! parser_errposition(@5))); } | decl_varname K_ALIAS K_FOR decl_aliasitem ';' { *************** decl_cursor_args : *** 632,637 **** --- 615,621 ---- foreach (l, $2) { PLpgSQL_variable *arg = (PLpgSQL_variable *) lfirst(l); + Assert(!arg->isconst); new->fieldnames[i] = arg->refname; new->varnos[i] = arg->dno; i++; *************** for_control : for_variable K_IN *** 1378,1383 **** --- 1362,1368 ---- new->var = (PLpgSQL_variable *) plpgsql_build_record($1.name, $1.lineno, + NULL, RECORDOID, true); *************** exception_sect : *** 2205,2211 **** -1, plpgsql_curr_compile->fn_input_collation), true); ! ((PLpgSQL_var *) var)->isconst = true; new->sqlstate_varno = var->dno; var = plpgsql_build_variable("sqlerrm", lineno, --- 2190,2196 ---- -1, plpgsql_curr_compile->fn_input_collation), true); ! var->isconst = true; new->sqlstate_varno = var->dno; var = plpgsql_build_variable("sqlerrm", lineno, *************** exception_sect : *** 2213,2219 **** -1, plpgsql_curr_compile->fn_input_collation), true); ! ((PLpgSQL_var *) var)->isconst = true; new->sqlerrm_varno = var->dno; $<exception_block>$ = new; --- 2198,2204 ---- -1, plpgsql_curr_compile->fn_input_collation), true); ! var->isconst = true; new->sqlerrm_varno = var->dno; $<exception_block>$ = new; *************** check_assignable(PLpgSQL_datum *datum, i *** 3287,3310 **** { case PLPGSQL_DTYPE_VAR: case PLPGSQL_DTYPE_PROMISE: ! if (((PLpgSQL_var *) datum)->isconst) ereport(ERROR, (errcode(ERRCODE_ERROR_IN_ASSIGNMENT), ! errmsg("\"%s\" is declared CONSTANT", ! ((PLpgSQL_var *) datum)->refname), parser_errposition(location))); break; case PLPGSQL_DTYPE_ROW: ! /* always assignable? Shouldn't we check member vars? */ ! break; ! case PLPGSQL_DTYPE_REC: ! /* always assignable? What about NEW/OLD? */ break; case PLPGSQL_DTYPE_RECFIELD: ! /* always assignable? */ break; case PLPGSQL_DTYPE_ARRAYELEM: ! /* always assignable? */ break; default: elog(ERROR, "unrecognized dtype: %d", datum->dtype); --- 3272,3297 ---- { case PLPGSQL_DTYPE_VAR: case PLPGSQL_DTYPE_PROMISE: ! case PLPGSQL_DTYPE_REC: ! if (((PLpgSQL_variable *) datum)->isconst) ereport(ERROR, (errcode(ERRCODE_ERROR_IN_ASSIGNMENT), ! errmsg("variable \"%s\" is declared CONSTANT", ! ((PLpgSQL_variable *) datum)->refname), parser_errposition(location))); break; case PLPGSQL_DTYPE_ROW: ! /* always assignable; member vars were checked at compile time */ break; case PLPGSQL_DTYPE_RECFIELD: ! /* assignable if parent record is */ ! check_assignable(plpgsql_Datums[((PLpgSQL_recfield *) datum)->recparentno], ! location); break; case PLPGSQL_DTYPE_ARRAYELEM: ! /* assignable if parent array is */ ! check_assignable(plpgsql_Datums[((PLpgSQL_arrayelem *) datum)->arrayparentno], ! location); break; default: elog(ERROR, "unrecognized dtype: %d", datum->dtype); *************** read_into_scalar_list(char *initial_name *** 3429,3437 **** */ plpgsql_push_back_token(tok); ! row = palloc(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; - row->refname = pstrdup("*internal*"); row->lineno = plpgsql_location_to_lineno(initial_location); row->rowtupdesc = NULL; row->nfields = nfields; --- 3416,3423 ---- */ plpgsql_push_back_token(tok); ! row = palloc0(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; row->lineno = plpgsql_location_to_lineno(initial_location); row->rowtupdesc = NULL; row->nfields = nfields; *************** make_scalar_list1(char *initial_name, *** 3464,3472 **** check_assignable(initial_datum, location); ! row = palloc(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; - row->refname = pstrdup("*internal*"); row->lineno = lineno; row->rowtupdesc = NULL; row->nfields = 1; --- 3450,3457 ---- check_assignable(initial_datum, location); ! row = palloc0(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; row->lineno = lineno; row->rowtupdesc = NULL; row->nfields = 1; diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 96e92b2..c1c2b40 100644 *** a/src/pl/plpgsql/src/plpgsql.h --- b/src/pl/plpgsql/src/plpgsql.h *************** typedef struct PLpgSQL_variable *** 262,267 **** --- 262,270 ---- int dno; char *refname; int lineno; + bool isconst; + bool notnull; + PLpgSQL_expr *default_val; } PLpgSQL_variable; /* *************** typedef struct PLpgSQL_var *** 281,292 **** int dno; char *refname; int lineno; - /* end of PLpgSQL_variable fields */ - bool isconst; bool notnull; - PLpgSQL_type *datatype; PLpgSQL_expr *default_val; /* * Variables declared as CURSOR FOR <query> are mostly like ordinary --- 284,295 ---- int dno; char *refname; int lineno; bool isconst; bool notnull; PLpgSQL_expr *default_val; + /* end of PLpgSQL_variable fields */ + + PLpgSQL_type *datatype; /* * Variables declared as CURSOR FOR <query> are mostly like ordinary *************** typedef struct PLpgSQL_var *** 318,323 **** --- 321,331 ---- * * Note that there's no way to name the row as such from PL/pgSQL code, * so many functions don't need to support these. + * + * refname, isconst, notnull, and default_val are unsupported (and hence + * always zero/null) for a row. The member variables of a row should have + * been checked to be writable at compile time, so isconst is correctly set + * to false. notnull and default_val aren't applicable. */ typedef struct PLpgSQL_row { *************** typedef struct PLpgSQL_row *** 325,330 **** --- 333,341 ---- int dno; char *refname; int lineno; + bool isconst; + bool notnull; + PLpgSQL_expr *default_val; /* end of PLpgSQL_variable fields */ /* *************** typedef struct PLpgSQL_rec *** 348,358 **** --- 359,376 ---- int dno; char *refname; int lineno; + bool isconst; + bool notnull; + PLpgSQL_expr *default_val; /* end of PLpgSQL_variable fields */ + PLpgSQL_type *datatype; /* can be NULL, if rectypeid is RECORDOID */ Oid rectypeid; /* declared type of variable */ /* RECFIELDs for this record are chained together for easy access */ int firstfield; /* dno of first RECFIELD, or -1 if none */ + + /* Fields below here can change at runtime */ + /* We always store record variables as "expanded" records */ ExpandedRecordHeader *erh; } PLpgSQL_rec; *************** extern PLpgSQL_variable *plpgsql_build_v *** 1121,1127 **** PLpgSQL_type *dtype, bool add2namespace); extern PLpgSQL_rec *plpgsql_build_record(const char *refname, int lineno, ! Oid rectypeid, bool add2namespace); extern PLpgSQL_recfield *plpgsql_build_recfield(PLpgSQL_rec *rec, const char *fldname); extern int plpgsql_recognize_err_condition(const char *condname, --- 1139,1146 ---- PLpgSQL_type *dtype, bool add2namespace); extern PLpgSQL_rec *plpgsql_build_record(const char *refname, int lineno, ! PLpgSQL_type *dtype, Oid rectypeid, ! bool add2namespace); extern PLpgSQL_recfield *plpgsql_build_recfield(PLpgSQL_rec *rec, const char *fldname); extern int plpgsql_recognize_err_condition(const char *condname, diff --git a/src/pl/plpgsql/src/sql/plpgsql_varprops.sql b/src/pl/plpgsql/src/sql/plpgsql_varprops.sql index ...c0e7f95 . *** a/src/pl/plpgsql/src/sql/plpgsql_varprops.sql --- b/src/pl/plpgsql/src/sql/plpgsql_varprops.sql *************** *** 0 **** --- 1,249 ---- + -- + -- Tests for PL/pgSQL variable properties: CONSTANT, NOT NULL, initializers + -- + + create type var_record as (f1 int4, f2 int4); + create domain int_nn as int not null; + create domain var_record_nn as var_record not null; + create domain var_record_colnn as var_record check((value).f2 is not null); + + -- CONSTANT + + do $$ + declare x constant int := 42; + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x constant int; + begin + x := 42; -- fail + end$$; + + do $$ + declare x constant int; y int; + begin + for x, y in select 1, 2 loop -- fail + end loop; + end$$; + + do $$ + declare x constant int[]; + begin + x[1] := 42; -- fail + end$$; + + do $$ + declare x constant int[]; y int; + begin + for x[1], y in select 1, 2 loop -- fail (currently, unsupported syntax) + end loop; + end$$; + + do $$ + declare x constant var_record; + begin + x.f1 := 42; -- fail + end$$; + + do $$ + declare x constant var_record; y int; + begin + for x.f1, y in select 1, 2 loop -- fail + end loop; + end$$; + + -- initializer expressions + + do $$ + declare x int := sin(0); + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int := 1/0; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x bigint[] := array[1,3,5]; + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x record := row(1,2,3); + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record := row(1,2); + begin + raise notice 'x = %', x; + end$$; + + -- NOT NULL + + do $$ + declare x int not null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int not null := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x int not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x record not null := row(42); + begin + raise notice 'x = %', x; + x := row(null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record not null := row(41,42); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x var_record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + -- Check that variables are reinitialized on block re-entry. + + \set VERBOSITY terse \\ -- needed for output stability + do $$ + begin + for i in 1..3 loop + declare + x int; + y int := i; + r record; + c var_record; + begin + if i = 1 then + x := 42; + r := row(i, i+1); + c := row(i, i+1); + end if; + raise notice 'x = %', x; + raise notice 'y = %', y; + raise notice 'r = %', r; + raise notice 'c = %', c; + end; + end loop; + end$$; + \set VERBOSITY default + + -- Check enforcement of domain constraints during initialization + + do $$ + declare x int_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int_nn := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x var_record_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_nn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + x := null; -- fail + end$$; + + do $$ + declare x var_record_colnn; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_colnn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_colnn := row(1,null); -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- fail + end$$; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index e38fe90..bb35326 100644 *** a/src/test/regress/expected/plpgsql.out --- b/src/test/regress/expected/plpgsql.out *************** select scope_test(); *** 5025,5066 **** (1 row) drop function scope_test(); - -- Check that variables are reinitialized on block re-entry. - \set VERBOSITY terse \\ -- needed for output stability - do $$ - begin - for i in 1..3 loop - declare - x int; - y int := i; - r record; - c int8_tbl; - begin - if i = 1 then - x := 42; - r := row(i, i+1); - c := row(i, i+1); - end if; - raise notice 'x = %', x; - raise notice 'y = %', y; - raise notice 'r = %', r; - raise notice 'c = %', c; - end; - end loop; - end$$; - NOTICE: x = 42 - NOTICE: y = 1 - NOTICE: r = (1,2) - NOTICE: c = (1,2) - NOTICE: x = <NULL> - NOTICE: y = 2 - NOTICE: r = <NULL> - NOTICE: c = <NULL> - NOTICE: x = <NULL> - NOTICE: y = 3 - NOTICE: r = <NULL> - NOTICE: c = <NULL> - \set VERBOSITY default -- Check handling of conflicts between plpgsql vars and table columns. set plpgsql.variable_conflict = error; create function conflict_test() returns setof int8_tbl as $$ --- 5025,5030 ---- diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 132f912..6620ea6 100644 *** a/src/test/regress/sql/plpgsql.sql --- b/src/test/regress/sql/plpgsql.sql *************** select scope_test(); *** 4014,4045 **** drop function scope_test(); - -- Check that variables are reinitialized on block re-entry. - - \set VERBOSITY terse \\ -- needed for output stability - do $$ - begin - for i in 1..3 loop - declare - x int; - y int := i; - r record; - c int8_tbl; - begin - if i = 1 then - x := 42; - r := row(i, i+1); - c := row(i, i+1); - end if; - raise notice 'x = %', x; - raise notice 'y = %', y; - raise notice 'r = %', r; - raise notice 'c = %', c; - end; - end loop; - end$$; - \set VERBOSITY default - -- Check handling of conflicts between plpgsql vars and table columns. set plpgsql.variable_conflict = error; --- 4014,4019 ----
I wrote: > I said a couple of times in recent threads that it wouldn't be too hard > to implement $SUBJECT given the other patches I've been working on. Here's a version rebased up to HEAD, with a trivial merge conflict fixed. This now needs to be applied over the patches in https://postgr.es/m/833.1516834367@sss.pgh.pa.us and https://postgr.es/m/8376.1516835784@sss.pgh.pa.us regards, tom lane diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile index 2190eab..3ac64e2 100644 *** a/src/pl/plpgsql/src/Makefile --- b/src/pl/plpgsql/src/Makefile *************** DATA = plpgsql.control plpgsql--1.0.sql *** 26,32 **** REGRESS_OPTS = --dbname=$(PL_TESTDB) ! REGRESS = plpgsql_call plpgsql_control plpgsql_record plpgsql_transaction all: all-lib --- 26,33 ---- REGRESS_OPTS = --dbname=$(PL_TESTDB) ! REGRESS = plpgsql_call plpgsql_control plpgsql_record \ ! plpgsql_transaction plpgsql_varprops all: all-lib diff --git a/src/pl/plpgsql/src/expected/plpgsql_varprops.out b/src/pl/plpgsql/src/expected/plpgsql_varprops.out index ...109056c . *** a/src/pl/plpgsql/src/expected/plpgsql_varprops.out --- b/src/pl/plpgsql/src/expected/plpgsql_varprops.out *************** *** 0 **** --- 1,300 ---- + -- + -- Tests for PL/pgSQL variable properties: CONSTANT, NOT NULL, initializers + -- + create type var_record as (f1 int4, f2 int4); + create domain int_nn as int not null; + create domain var_record_nn as var_record not null; + create domain var_record_colnn as var_record check((value).f2 is not null); + -- CONSTANT + do $$ + declare x constant int := 42; + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = 42 + do $$ + declare x constant int; + begin + x := 42; -- fail + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: x := 42; -- fail + ^ + do $$ + declare x constant int; y int; + begin + for x, y in select 1, 2 loop -- fail + end loop; + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: for x, y in select 1, 2 loop -- fail + ^ + do $$ + declare x constant int[]; + begin + x[1] := 42; -- fail + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: x[1] := 42; -- fail + ^ + do $$ + declare x constant int[]; y int; + begin + for x[1], y in select 1, 2 loop -- fail (currently, unsupported syntax) + end loop; + end$$; + ERROR: syntax error at or near "[" + LINE 4: for x[1], y in select 1, 2 loop -- fail (currently, unsup... + ^ + do $$ + declare x constant var_record; + begin + x.f1 := 42; -- fail + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: x.f1 := 42; -- fail + ^ + do $$ + declare x constant var_record; y int; + begin + for x.f1, y in select 1, 2 loop -- fail + end loop; + end$$; + ERROR: variable "x" is declared CONSTANT + LINE 4: for x.f1, y in select 1, 2 loop -- fail + ^ + -- initializer expressions + do $$ + declare x int := sin(0); + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = 0 + do $$ + declare x int := 1/0; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: division by zero + CONTEXT: SQL statement "SELECT 1/0" + PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x bigint[] := array[1,3,5]; + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = {1,3,5} + do $$ + declare x record := row(1,2,3); + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = (1,2,3) + do $$ + declare x var_record := row(1,2); + begin + raise notice 'x = %', x; + end$$; + NOTICE: x = (1,2) + -- NOT NULL + do $$ + declare x int not null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: variable "x" must have a default value, since it's declared NOT NULL + LINE 2: declare x int not null; -- fail + ^ + do $$ + declare x int not null := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = 42 + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment + do $$ + declare x int not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: variable "x" must have a default value, since it's declared NOT NULL + LINE 2: declare x record not null; -- fail + ^ + do $$ + declare x record not null := row(42); + begin + raise notice 'x = %', x; + x := row(null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = (42) + NOTICE: x = () + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 7 at assignment + do $$ + declare x record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: variable "x" must have a default value, since it's declared NOT NULL + LINE 2: declare x var_record not null; -- fail + ^ + do $$ + declare x var_record not null := row(41,42); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = (41,42) + NOTICE: x = (,) + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 7 at assignment + do $$ + declare x var_record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: null value cannot be assigned to variable "x" declared NOT NULL + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + -- Check that variables are reinitialized on block re-entry. + \set VERBOSITY terse \\ -- needed for output stability + do $$ + begin + for i in 1..3 loop + declare + x int; + y int := i; + r record; + c var_record; + begin + if i = 1 then + x := 42; + r := row(i, i+1); + c := row(i, i+1); + end if; + raise notice 'x = %', x; + raise notice 'y = %', y; + raise notice 'r = %', r; + raise notice 'c = %', c; + end; + end loop; + end$$; + NOTICE: x = 42 + NOTICE: y = 1 + NOTICE: r = (1,2) + NOTICE: c = (1,2) + NOTICE: x = <NULL> + NOTICE: y = 2 + NOTICE: r = <NULL> + NOTICE: c = <NULL> + NOTICE: x = <NULL> + NOTICE: y = 3 + NOTICE: r = <NULL> + NOTICE: c = <NULL> + \set VERBOSITY default + -- Check enforcement of domain constraints during initialization + do $$ + declare x int_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain int_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x int_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain int_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x int_nn := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = 42 + ERROR: domain int_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment + do $$ + declare x var_record_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain var_record_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: domain var_record_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_nn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + x := null; -- fail + end$$; + NOTICE: x = (1,2) + ERROR: domain var_record_nn does not allow null values + CONTEXT: PL/pgSQL function inline_code_block line 6 at assignment + do $$ + declare x var_record_colnn; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_colnn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_colnn := row(1,null); -- fail + begin + raise notice 'x = %', x; + end$$; + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 3 during statement block local variable initialization + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + NOTICE: x = (1,2) + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- fail + end$$; + NOTICE: x = (1,2) + ERROR: value for domain var_record_colnn violates check constraint "var_record_colnn_check" + CONTEXT: PL/pgSQL function inline_code_block line 5 at assignment diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 526aa8f..aab92c4 100644 *** a/src/pl/plpgsql/src/pl_comp.c --- b/src/pl/plpgsql/src/pl_comp.c *************** do_compile(FunctionCallInfo fcinfo, *** 594,604 **** errhint("The arguments of the trigger can be accessed through TG_NARGS and TG_ARGV instead."))); /* Add the record for referencing NEW ROW */ ! rec = plpgsql_build_record("new", 0, RECORDOID, true); function->new_varno = rec->dno; /* Add the record for referencing OLD ROW */ ! rec = plpgsql_build_record("old", 0, RECORDOID, true); function->old_varno = rec->dno; /* Add the variable tg_name */ --- 594,604 ---- errhint("The arguments of the trigger can be accessed through TG_NARGS and TG_ARGV instead."))); /* Add the record for referencing NEW ROW */ ! rec = plpgsql_build_record("new", 0, NULL, RECORDOID, true); function->new_varno = rec->dno; /* Add the record for referencing OLD ROW */ ! rec = plpgsql_build_record("old", 0, NULL, RECORDOID, true); function->old_varno = rec->dno; /* Add the variable tg_name */ *************** plpgsql_build_variable(const char *refna *** 1811,1817 **** var->refname = pstrdup(refname); var->lineno = lineno; var->datatype = dtype; ! /* other fields might be filled by caller */ /* preset to NULL */ var->value = 0; --- 1811,1817 ---- var->refname = pstrdup(refname); var->lineno = lineno; var->datatype = dtype; ! /* other fields are left as 0, might be changed by caller */ /* preset to NULL */ var->value = 0; *************** plpgsql_build_variable(const char *refna *** 1831,1837 **** /* Composite type -- build a record variable */ PLpgSQL_rec *rec; ! rec = plpgsql_build_record(refname, lineno, dtype->typoid, add2namespace); result = (PLpgSQL_variable *) rec; break; --- 1831,1838 ---- /* Composite type -- build a record variable */ PLpgSQL_rec *rec; ! rec = plpgsql_build_record(refname, lineno, ! dtype, dtype->typoid, add2namespace); result = (PLpgSQL_variable *) rec; break; *************** plpgsql_build_variable(const char *refna *** 1856,1862 **** * Build empty named record variable, and optionally add it to namespace */ PLpgSQL_rec * ! plpgsql_build_record(const char *refname, int lineno, Oid rectypeid, bool add2namespace) { PLpgSQL_rec *rec; --- 1857,1864 ---- * Build empty named record variable, and optionally add it to namespace */ PLpgSQL_rec * ! plpgsql_build_record(const char *refname, int lineno, ! PLpgSQL_type *dtype, Oid rectypeid, bool add2namespace) { PLpgSQL_rec *rec; *************** plpgsql_build_record(const char *refname *** 1865,1870 **** --- 1867,1874 ---- rec->dtype = PLPGSQL_DTYPE_REC; rec->refname = pstrdup(refname); rec->lineno = lineno; + /* other fields are left as 0, might be changed by caller */ + rec->datatype = dtype; rec->rectypeid = rectypeid; rec->firstfield = -1; rec->erh = NULL; *************** build_row_from_vars(PLpgSQL_variable **v *** 1899,1904 **** --- 1903,1911 ---- int32 typmod; Oid typcoll; + /* Member vars of a row should never be const */ + Assert(!var->isconst); + switch (var->dtype) { case PLPGSQL_DTYPE_VAR: diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index d578c56..62010d8 100644 *** a/src/pl/plpgsql/src/pl_exec.c --- b/src/pl/plpgsql/src/pl_exec.c *************** plpgsql_exec_function(PLpgSQL_function * *** 539,545 **** } else { ! /* If arg is null, treat it as an empty row */ exec_move_row(&estate, (PLpgSQL_variable *) rec, NULL, NULL); } --- 539,545 ---- } else { ! /* If arg is null, set variable to null */ exec_move_row(&estate, (PLpgSQL_variable *) rec, NULL, NULL); } *************** exec_stmt_block(PLpgSQL_execstate *estat *** 1539,1549 **** { /* * If needed, give the datatype a chance to reject ! * NULLs, by assigning a NULL to the variable. We * claim the value is of type UNKNOWN, not the var's ! * datatype, else coercion will be skipped. (Do this ! * before the notnull check to be consistent with ! * exec_assign_value.) */ if (var->datatype->typtype == TYPTYPE_DOMAIN) exec_assign_value(estate, --- 1539,1547 ---- { /* * If needed, give the datatype a chance to reject ! * NULLs, by assigning a NULL to the variable. We * claim the value is of type UNKNOWN, not the var's ! * datatype, else coercion will be skipped. */ if (var->datatype->typtype == TYPTYPE_DOMAIN) exec_assign_value(estate, *************** exec_stmt_block(PLpgSQL_execstate *estat *** 1553,1563 **** UNKNOWNOID, -1); ! if (var->notnull) ! ereport(ERROR, ! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), ! errmsg("variable \"%s\" declared NOT NULL cannot default to NULL", ! var->refname))); } else { --- 1551,1558 ---- UNKNOWNOID, -1); ! /* parser should have rejected NOT NULL */ ! Assert(!var->notnull); } else { *************** exec_stmt_block(PLpgSQL_execstate *estat *** 1571,1579 **** { PLpgSQL_rec *rec = (PLpgSQL_rec *) datum; ! if (rec->erh) ! DeleteExpandedObject(ExpandedRecordGetDatum(rec->erh)); ! rec->erh = NULL; } break; --- 1566,1593 ---- { PLpgSQL_rec *rec = (PLpgSQL_rec *) datum; ! /* ! * Deletion of any existing object will be handled during ! * the assignments below, and in some cases it's more ! * efficient for us not to get rid of it beforehand. ! */ ! if (rec->default_val == NULL) ! { ! /* ! * If needed, give the datatype a chance to reject ! * NULLs, by assigning a NULL to the variable. ! */ ! exec_move_row(estate, (PLpgSQL_variable *) rec, ! NULL, NULL); ! ! /* parser should have rejected NOT NULL */ ! Assert(!rec->notnull); ! } ! else ! { ! exec_assign_expr(estate, (PLpgSQL_datum *) rec, ! rec->default_val); ! } } break; *************** exec_assign_value(PLpgSQL_execstate *est *** 4725,4731 **** if (isNull) { ! /* If source is null, just assign nulls to the record */ exec_move_row(estate, (PLpgSQL_variable *) rec, NULL, NULL); } --- 4739,4751 ---- if (isNull) { ! if (rec->notnull) ! ereport(ERROR, ! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), ! errmsg("null value cannot be assigned to variable \"%s\" declared NOT NULL", ! rec->refname))); ! ! /* Set variable to a simple NULL */ exec_move_row(estate, (PLpgSQL_variable *) rec, NULL, NULL); } *************** exec_move_row(PLpgSQL_execstate *estate, *** 6375,6383 **** */ if (tupdesc == NULL) { ! if (rec->erh) ! DeleteExpandedObject(ExpandedRecordGetDatum(rec->erh)); ! rec->erh = NULL; return; } --- 6395,6421 ---- */ if (tupdesc == NULL) { ! if (rec->datatype && ! rec->datatype->typtype == TYPTYPE_DOMAIN) ! { ! /* ! * If it's a composite domain, NULL might not be a legal ! * value, so we instead need to make an empty expanded record ! * and ensure that domain type checking gets done. If there ! * is already an expanded record, piggyback on its lookups. ! */ ! newerh = make_expanded_record_for_rec(estate, rec, ! NULL, rec->erh); ! expanded_record_set_tuple(newerh, NULL, false); ! assign_record_var(estate, rec, newerh); ! } ! else ! { ! /* Just clear it to NULL */ ! if (rec->erh) ! DeleteExpandedObject(ExpandedRecordGetDatum(rec->erh)); ! rec->erh = NULL; ! } return; } diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c index 379fd69..b986fc3 100644 *** a/src/pl/plpgsql/src/pl_funcs.c --- b/src/pl/plpgsql/src/pl_funcs.c *************** plpgsql_free_function_memory(PLpgSQL_fun *** 740,745 **** --- 740,750 ---- case PLPGSQL_DTYPE_ROW: break; case PLPGSQL_DTYPE_REC: + { + PLpgSQL_rec *rec = (PLpgSQL_rec *) d; + + free_expr(rec->default_val); + } break; case PLPGSQL_DTYPE_RECFIELD: break; *************** plpgsql_dumptree(PLpgSQL_function *func) *** 1633,1638 **** --- 1638,1653 ---- printf("REC %-16s typoid %u\n", ((PLpgSQL_rec *) d)->refname, ((PLpgSQL_rec *) d)->rectypeid); + if (((PLpgSQL_rec *) d)->isconst) + printf(" CONSTANT\n"); + if (((PLpgSQL_rec *) d)->notnull) + printf(" NOT NULL\n"); + if (((PLpgSQL_rec *) d)->default_val != NULL) + { + printf(" DEFAULT "); + dump_expr(((PLpgSQL_rec *) d)->default_val); + printf("\n"); + } break; case PLPGSQL_DTYPE_RECFIELD: printf("RECFIELD %-16s of REC %d\n", diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index 5bf4594..688fbd6 100644 *** a/src/pl/plpgsql/src/pl_gram.y --- b/src/pl/plpgsql/src/pl_gram.y *************** decl_statement : decl_varname decl_const *** 505,541 **** var = plpgsql_build_variable($1.name, $1.lineno, $3, true); ! if ($2) ! { ! if (var->dtype == PLPGSQL_DTYPE_VAR) ! ((PLpgSQL_var *) var)->isconst = $2; ! else ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("record variable cannot be CONSTANT"), ! parser_errposition(@2))); ! } ! if ($5) ! { ! if (var->dtype == PLPGSQL_DTYPE_VAR) ! ((PLpgSQL_var *) var)->notnull = $5; ! else ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("record variable cannot be NOT NULL"), ! parser_errposition(@4))); ! } ! if ($6 != NULL) ! { ! if (var->dtype == PLPGSQL_DTYPE_VAR) ! ((PLpgSQL_var *) var)->default_val = $6; ! else ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("default value for record variable is not supported"), ! parser_errposition(@5))); ! } } | decl_varname K_ALIAS K_FOR decl_aliasitem ';' { --- 505,524 ---- var = plpgsql_build_variable($1.name, $1.lineno, $3, true); ! var->isconst = $2; ! var->notnull = $5; ! var->default_val = $6; ! /* ! * The combination of NOT NULL without an initializer ! * can't work, so let's reject it at compile time. ! */ ! if (var->notnull && var->default_val == NULL) ! ereport(ERROR, ! (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), ! errmsg("variable \"%s\" must have a default value, since it's declared NOT NULL", ! var->refname), ! parser_errposition(@5))); } | decl_varname K_ALIAS K_FOR decl_aliasitem ';' { *************** decl_cursor_args : *** 635,640 **** --- 618,624 ---- foreach (l, $2) { PLpgSQL_variable *arg = (PLpgSQL_variable *) lfirst(l); + Assert(!arg->isconst); new->fieldnames[i] = arg->refname; new->varnos[i] = arg->dno; i++; *************** for_control : for_variable K_IN *** 1385,1390 **** --- 1369,1375 ---- new->var = (PLpgSQL_variable *) plpgsql_build_record($1.name, $1.lineno, + NULL, RECORDOID, true); *************** exception_sect : *** 2237,2243 **** -1, plpgsql_curr_compile->fn_input_collation), true); ! ((PLpgSQL_var *) var)->isconst = true; new->sqlstate_varno = var->dno; var = plpgsql_build_variable("sqlerrm", lineno, --- 2222,2228 ---- -1, plpgsql_curr_compile->fn_input_collation), true); ! var->isconst = true; new->sqlstate_varno = var->dno; var = plpgsql_build_variable("sqlerrm", lineno, *************** exception_sect : *** 2245,2251 **** -1, plpgsql_curr_compile->fn_input_collation), true); ! ((PLpgSQL_var *) var)->isconst = true; new->sqlerrm_varno = var->dno; $<exception_block>$ = new; --- 2230,2236 ---- -1, plpgsql_curr_compile->fn_input_collation), true); ! var->isconst = true; new->sqlerrm_varno = var->dno; $<exception_block>$ = new; *************** check_assignable(PLpgSQL_datum *datum, i *** 3321,3344 **** { case PLPGSQL_DTYPE_VAR: case PLPGSQL_DTYPE_PROMISE: ! if (((PLpgSQL_var *) datum)->isconst) ereport(ERROR, (errcode(ERRCODE_ERROR_IN_ASSIGNMENT), ! errmsg("\"%s\" is declared CONSTANT", ! ((PLpgSQL_var *) datum)->refname), parser_errposition(location))); break; case PLPGSQL_DTYPE_ROW: ! /* always assignable? Shouldn't we check member vars? */ ! break; ! case PLPGSQL_DTYPE_REC: ! /* always assignable? What about NEW/OLD? */ break; case PLPGSQL_DTYPE_RECFIELD: ! /* always assignable? */ break; case PLPGSQL_DTYPE_ARRAYELEM: ! /* always assignable? */ break; default: elog(ERROR, "unrecognized dtype: %d", datum->dtype); --- 3306,3331 ---- { case PLPGSQL_DTYPE_VAR: case PLPGSQL_DTYPE_PROMISE: ! case PLPGSQL_DTYPE_REC: ! if (((PLpgSQL_variable *) datum)->isconst) ereport(ERROR, (errcode(ERRCODE_ERROR_IN_ASSIGNMENT), ! errmsg("variable \"%s\" is declared CONSTANT", ! ((PLpgSQL_variable *) datum)->refname), parser_errposition(location))); break; case PLPGSQL_DTYPE_ROW: ! /* always assignable; member vars were checked at compile time */ break; case PLPGSQL_DTYPE_RECFIELD: ! /* assignable if parent record is */ ! check_assignable(plpgsql_Datums[((PLpgSQL_recfield *) datum)->recparentno], ! location); break; case PLPGSQL_DTYPE_ARRAYELEM: ! /* assignable if parent array is */ ! check_assignable(plpgsql_Datums[((PLpgSQL_arrayelem *) datum)->arrayparentno], ! location); break; default: elog(ERROR, "unrecognized dtype: %d", datum->dtype); *************** read_into_scalar_list(char *initial_name *** 3463,3471 **** */ plpgsql_push_back_token(tok); ! row = palloc(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; - row->refname = pstrdup("*internal*"); row->lineno = plpgsql_location_to_lineno(initial_location); row->rowtupdesc = NULL; row->nfields = nfields; --- 3450,3457 ---- */ plpgsql_push_back_token(tok); ! row = palloc0(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; row->lineno = plpgsql_location_to_lineno(initial_location); row->rowtupdesc = NULL; row->nfields = nfields; *************** make_scalar_list1(char *initial_name, *** 3498,3506 **** check_assignable(initial_datum, location); ! row = palloc(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; - row->refname = pstrdup("*internal*"); row->lineno = lineno; row->rowtupdesc = NULL; row->nfields = 1; --- 3484,3491 ---- check_assignable(initial_datum, location); ! row = palloc0(sizeof(PLpgSQL_row)); row->dtype = PLPGSQL_DTYPE_ROW; row->lineno = lineno; row->rowtupdesc = NULL; row->nfields = 1; diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 01b89a5..c2449f0 100644 *** a/src/pl/plpgsql/src/plpgsql.h --- b/src/pl/plpgsql/src/plpgsql.h *************** typedef struct PLpgSQL_variable *** 264,269 **** --- 264,272 ---- int dno; char *refname; int lineno; + bool isconst; + bool notnull; + PLpgSQL_expr *default_val; } PLpgSQL_variable; /* *************** typedef struct PLpgSQL_var *** 283,294 **** int dno; char *refname; int lineno; - /* end of PLpgSQL_variable fields */ - bool isconst; bool notnull; - PLpgSQL_type *datatype; PLpgSQL_expr *default_val; /* * Variables declared as CURSOR FOR <query> are mostly like ordinary --- 286,297 ---- int dno; char *refname; int lineno; bool isconst; bool notnull; PLpgSQL_expr *default_val; + /* end of PLpgSQL_variable fields */ + + PLpgSQL_type *datatype; /* * Variables declared as CURSOR FOR <query> are mostly like ordinary *************** typedef struct PLpgSQL_var *** 320,325 **** --- 323,333 ---- * * Note that there's no way to name the row as such from PL/pgSQL code, * so many functions don't need to support these. + * + * refname, isconst, notnull, and default_val are unsupported (and hence + * always zero/null) for a row. The member variables of a row should have + * been checked to be writable at compile time, so isconst is correctly set + * to false. notnull and default_val aren't applicable. */ typedef struct PLpgSQL_row { *************** typedef struct PLpgSQL_row *** 327,332 **** --- 335,343 ---- int dno; char *refname; int lineno; + bool isconst; + bool notnull; + PLpgSQL_expr *default_val; /* end of PLpgSQL_variable fields */ /* *************** typedef struct PLpgSQL_rec *** 350,360 **** --- 361,378 ---- int dno; char *refname; int lineno; + bool isconst; + bool notnull; + PLpgSQL_expr *default_val; /* end of PLpgSQL_variable fields */ + PLpgSQL_type *datatype; /* can be NULL, if rectypeid is RECORDOID */ Oid rectypeid; /* declared type of variable */ /* RECFIELDs for this record are chained together for easy access */ int firstfield; /* dno of first RECFIELD, or -1 if none */ + + /* Fields below here can change at runtime */ + /* We always store record variables as "expanded" records */ ExpandedRecordHeader *erh; } PLpgSQL_rec; *************** extern PLpgSQL_variable *plpgsql_build_v *** 1141,1147 **** PLpgSQL_type *dtype, bool add2namespace); extern PLpgSQL_rec *plpgsql_build_record(const char *refname, int lineno, ! Oid rectypeid, bool add2namespace); extern PLpgSQL_recfield *plpgsql_build_recfield(PLpgSQL_rec *rec, const char *fldname); extern int plpgsql_recognize_err_condition(const char *condname, --- 1159,1166 ---- PLpgSQL_type *dtype, bool add2namespace); extern PLpgSQL_rec *plpgsql_build_record(const char *refname, int lineno, ! PLpgSQL_type *dtype, Oid rectypeid, ! bool add2namespace); extern PLpgSQL_recfield *plpgsql_build_recfield(PLpgSQL_rec *rec, const char *fldname); extern int plpgsql_recognize_err_condition(const char *condname, diff --git a/src/pl/plpgsql/src/sql/plpgsql_varprops.sql b/src/pl/plpgsql/src/sql/plpgsql_varprops.sql index ...c0e7f95 . *** a/src/pl/plpgsql/src/sql/plpgsql_varprops.sql --- b/src/pl/plpgsql/src/sql/plpgsql_varprops.sql *************** *** 0 **** --- 1,249 ---- + -- + -- Tests for PL/pgSQL variable properties: CONSTANT, NOT NULL, initializers + -- + + create type var_record as (f1 int4, f2 int4); + create domain int_nn as int not null; + create domain var_record_nn as var_record not null; + create domain var_record_colnn as var_record check((value).f2 is not null); + + -- CONSTANT + + do $$ + declare x constant int := 42; + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x constant int; + begin + x := 42; -- fail + end$$; + + do $$ + declare x constant int; y int; + begin + for x, y in select 1, 2 loop -- fail + end loop; + end$$; + + do $$ + declare x constant int[]; + begin + x[1] := 42; -- fail + end$$; + + do $$ + declare x constant int[]; y int; + begin + for x[1], y in select 1, 2 loop -- fail (currently, unsupported syntax) + end loop; + end$$; + + do $$ + declare x constant var_record; + begin + x.f1 := 42; -- fail + end$$; + + do $$ + declare x constant var_record; y int; + begin + for x.f1, y in select 1, 2 loop -- fail + end loop; + end$$; + + -- initializer expressions + + do $$ + declare x int := sin(0); + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int := 1/0; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x bigint[] := array[1,3,5]; + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x record := row(1,2,3); + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record := row(1,2); + begin + raise notice 'x = %', x; + end$$; + + -- NOT NULL + + do $$ + declare x int not null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int not null := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x int not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x record not null := row(42); + begin + raise notice 'x = %', x; + x := row(null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record not null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record not null := row(41,42); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x var_record not null := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + -- Check that variables are reinitialized on block re-entry. + + \set VERBOSITY terse \\ -- needed for output stability + do $$ + begin + for i in 1..3 loop + declare + x int; + y int := i; + r record; + c var_record; + begin + if i = 1 then + x := 42; + r := row(i, i+1); + c := row(i, i+1); + end if; + raise notice 'x = %', x; + raise notice 'y = %', y; + raise notice 'r = %', r; + raise notice 'c = %', c; + end; + end loop; + end$$; + \set VERBOSITY default + + -- Check enforcement of domain constraints during initialization + + do $$ + declare x int_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x int_nn := 42; + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x var_record_nn; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_nn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_nn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- ok + x := null; -- fail + end$$; + + do $$ + declare x var_record_colnn; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_colnn := null; -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_colnn := row(1,null); -- fail + begin + raise notice 'x = %', x; + end$$; + + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := null; -- fail + end$$; + + do $$ + declare x var_record_colnn := row(1,2); + begin + raise notice 'x = %', x; + x := row(null,null); -- fail + end$$; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 0c1da08..d294e53 100644 *** a/src/test/regress/expected/plpgsql.out --- b/src/test/regress/expected/plpgsql.out *************** select scope_test(); *** 4586,4627 **** (1 row) drop function scope_test(); - -- Check that variables are reinitialized on block re-entry. - \set VERBOSITY terse \\ -- needed for output stability - do $$ - begin - for i in 1..3 loop - declare - x int; - y int := i; - r record; - c int8_tbl; - begin - if i = 1 then - x := 42; - r := row(i, i+1); - c := row(i, i+1); - end if; - raise notice 'x = %', x; - raise notice 'y = %', y; - raise notice 'r = %', r; - raise notice 'c = %', c; - end; - end loop; - end$$; - NOTICE: x = 42 - NOTICE: y = 1 - NOTICE: r = (1,2) - NOTICE: c = (1,2) - NOTICE: x = <NULL> - NOTICE: y = 2 - NOTICE: r = <NULL> - NOTICE: c = <NULL> - NOTICE: x = <NULL> - NOTICE: y = 3 - NOTICE: r = <NULL> - NOTICE: c = <NULL> - \set VERBOSITY default -- Check handling of conflicts between plpgsql vars and table columns. set plpgsql.variable_conflict = error; create function conflict_test() returns setof int8_tbl as $$ --- 4586,4591 ---- diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 6bdcfe7..f17cf0b 100644 *** a/src/test/regress/sql/plpgsql.sql --- b/src/test/regress/sql/plpgsql.sql *************** select scope_test(); *** 3735,3766 **** drop function scope_test(); - -- Check that variables are reinitialized on block re-entry. - - \set VERBOSITY terse \\ -- needed for output stability - do $$ - begin - for i in 1..3 loop - declare - x int; - y int := i; - r record; - c int8_tbl; - begin - if i = 1 then - x := 42; - r := row(i, i+1); - c := row(i, i+1); - end if; - raise notice 'x = %', x; - raise notice 'y = %', y; - raise notice 'r = %', r; - raise notice 'c = %', c; - end; - end loop; - end$$; - \set VERBOSITY default - -- Check handling of conflicts between plpgsql vars and table columns. set plpgsql.variable_conflict = error; --- 3735,3740 ----
Re: CONSTANT/NOT NULL/initializer properties for plpgsql record variables
From
Daniel Gustafsson
Date:
> On 25 Jan 2018, at 00:32, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I wrote: >> I said a couple of times in recent threads that it wouldn't be too hard >> to implement $SUBJECT given the other patches I've been working on. > > Here's a version rebased up to HEAD, with a trivial merge conflict fixed. > > This now needs to be applied over the patches in > https://postgr.es/m/833.1516834367@sss.pgh.pa.us > and > https://postgr.es/m/8376.1516835784@sss.pgh.pa.us I’ve reviewed this patch (disclaimer: I did not review the patches listed above which it is based on) and the functionality introduced. The code is straight- forward, there are ample tests and I can’t make it break however many weird combinations thrown at it. Regarding the functionality it’s clearly a +1 on getting this in. One tiny thing: while not introduced in this patch, I wonder if it would be worth adding an errhint in the following hunk when applied to arrays, to clarify what CONSTANT in an array declaration mean. I have seen confusion around what y means in ‘x int[y]’, and I can see ‘x CONSTANT int[y]’ being misinterpreted as “length fixed to y”. - errmsg("\"%s\" is declared CONSTANT", - ((PLpgSQL_var *) datum)->refname), + errmsg("variable \"%s\" is declared CONSTANT", + ((PLpgSQL_variable *) datum)->refname), That might not be a common enough misunderstanding to warrant it, but it was the only thing that stood out to me (and perhaps it would be better in the docs if done at all). Setting this ready for committer, with the caveat that the underlying patches must go in of course. cheers ./daniel
Daniel Gustafsson <daniel@yesql.se> writes:
> I’ve reviewed this patch (disclaimer: I did not review the patches listed above
> which it is based on) and the functionality introduced. The code is straight-
> forward, there are ample tests and I can’t make it break however many weird
> combinations thrown at it. Regarding the functionality it’s clearly a +1 on
> getting this in.
Thanks for reviewing!
> One tiny thing: while not introduced in this patch, I wonder if it would be
> worth adding an errhint in the following hunk when applied to arrays, to
> clarify what CONSTANT in an array declaration mean. I have seen confusion
> around what y means in ‘x int[y]’, and I can see ‘x CONSTANT int[y]’ being
> misinterpreted as “length fixed to y”.
Hmm. What would you imagine the hint saying? It's certainly true that
a lot of people don't understand that a declared array length doesn't
mean anything in Postgres, but that's not specific to this context.
> That might not be a common enough misunderstanding to warrant it, but it was
> the only thing that stood out to me (and perhaps it would be better in the docs
> if done at all).
The documentation currently says
The CONSTANT option prevents the variable from being assigned to
after initialization, so that its value will remain constant for
the duration of the block.
I don't mind changing that if we can think of clearer wording, but to
me it seems pretty clear already.
regards, tom lane
CONSTANT/NOT NULL/initializer properties for plpgsql record variables
From
"David G. Johnston"
Date:
On Thursday, January 25, 2018, Tom Lane <tgl@sss.pgh.pa.us> wrote:
The documentation currently says
The CONSTANT option prevents the variable from being assigned to
after initialization, so that its value will remain constant for
the duration of the block.
While we don't really have the concept of mutable types other languages do. Maybe just saying "from being assigned or mutated after initialization" would suffice.
I don't see a desirable way to reinforce that the y in arr[y] is unenforced documentation - in the code or docs - beyond what is already done. That distinction doesn't usually come up and for those where it does the docs will probably be an after-the-fact confirmation of observed behavior.
David J.
Re: CONSTANT/NOT NULL/initializer properties for plpgsql record variables
From
Daniel Gustafsson
Date:
> On 25 Jan 2018, at 16:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Daniel Gustafsson <daniel@yesql.se> writes:
>> One tiny thing: while not introduced in this patch, I wonder if it would be
>> worth adding an errhint in the following hunk when applied to arrays, to
>> clarify what CONSTANT in an array declaration mean. I have seen confusion
>> around what y means in ‘x int[y]’, and I can see ‘x CONSTANT int[y]’ being
>> misinterpreted as “length fixed to y”.
>
> Hmm. What would you imagine the hint saying? It's certainly true that
> a lot of people don't understand that a declared array length doesn't
> mean anything in Postgres, but that's not specific to this context.
I was thinking about something along the lines of:
errhint("In array declarations, CONSTANT refers to elements and not length."),
.. when datatype->typisarray is true, but again it might just be waffling that
doesn’t help anyone.
cheers ./daniel