Hi,
I took a look at the rebased patch.
+ <entry><structfield>varisnotnull</structfield></entry>
+ <entry><type>boolean</type></entry>
+ <entry></entry>
+ <entry>
+ True if the schema variable doesn't allow null value. The default
value is false.
I wonder whether the field can be named in positive tense: e.g.
varallowsnull with default of true.
+ <entry><structfield>vareoxaction</structfield></entry>
+ <literal>n</literal> = no action, <literal>d</literal> = drop the
variable,
+ <literal>r</literal> = reset the variable to its default value.
Looks like there is only one action allowed. I wonder if there is a
possibility of having two actions at the same time in the future.
+ The <application>PL/pgSQL</application> language has not packages
+ and then it has not package variables and package constants. The
'has not' -> 'has no'
+ a null value. A variable created as NOT NULL and without an
explicitely
explicitely -> explicitly
+ int nnewmembers;
+ Oid *oldmembers;
+ Oid *newmembers;
I wonder if naming nnewmembers newmembercount would be more readable.
For pg_variable_aclcheck:
+ return ACLCHECK_OK;
+ else
+ return ACLCHECK_NO_PRIV;
The 'else' can be omitted.
+ * Ownership check for a schema variables (specified by OID).
'a schema variable' (no s)
For NamesFromList():
+ if (IsA(n, String))
+ {
+ result = lappend(result, n);
+ }
+ else
+ break;
There would be no more name if current n is not a String ?
+ * both variants, and returns InvalidOid with not_uniq flag,
when
'and return' (no s)
+ return InvalidOid;
+ }
+ else if (OidIsValid(varoid_without_attr))
'else' is not needed (since the if block ends with return).
For clean_cache_callback(),
+ if (hash_search(schemavarhashtab,
+ (void *) &svar->varid,
+ HASH_REMOVE,
+ NULL) == NULL)
+ elog(DEBUG1, "hash table corrupted");
Maybe add more information to the debug, such as var name.
Should we come out of the while loop in this scenario ?
Cheers
