Thanks for the update, smaller patches looks promising.
Off the list Pavel has mentioned that the first two patches contain a
bare minimum for session variables, so I've reviewed them once more and
suggest to concentrate on them first. I'm afraid the memory cleanup
patch has to be added to the "bare minimum" set as well -- otherwise in
my tests it was too easy to run out of memory via creating, assigning
and dropping variables. Unfortunately one can't extract those three
patches from the series and apply only them, the memory patch would have
some conflicts. Can you maybe reshuffle the series to have those patches
(1, 2 + 8) as first three?
If that's possible, my proposal would be to proceed with them first. To the
best of my knowledge they look good to me, except few minor details:
* The documentation says in a couple of places (ddl.sgml,
create_variable.sgml) that "Retrieving a session variable's value
returns either a NULL or a default value", but as far as I see the
default value feature is not implemented within first two patches.
* Similar with mentioning immutable session variables in plpgsql.sgml .
* Commentary to LookupVariable mentions a rowtype_only argument:
+/*
+ * Returns oid of session variable specified by possibly qualified identifier.
+ *
+ * If not found, returns InvalidOid if missing_ok, else throws error.
+ * When rowtype_only argument is true the session variables of not
+ * composite types are ignored. This should to reduce possible collisions.
+ */
+Oid
+LookupVariable(const char *nspname,
+ const char *varname,
+ bool missing_ok)
but the function doesn't have it.
* I've noticed an interesting result when a LET statement is used to assign a
value without a subquery:
create variable test as text;
-- returns NULL
select test;
-- use repeat directly without a subquery
let test = repeat("test", 100000);
-- returns NULL
select test;
I was expecting to see an error here, is this a correct behaviour?
