I’ve been burned my this issue in the past so would be great if this could get in.
+/*
+ * DomainHasVolatileConstraints --- check if a domain has constraints with
+ * volatile expressions
+ *
+ * Returns true if the domain has any constraints at all. If have_volatile
+ * is not NULL, also checks whether any CHECK constraint contains a volatile
+ * expression and sets *have_volatile accordingly.
+ *
+ * The caller must initialize *have_volatile before calling (typically to
+ * false). This function only ever sets it to true, never to false.
+ *
+ * This is defined to return false, not fail, if type is not a domain.
+ */
+bool
+DomainHasVolatileConstraints(Oid type_id, bool *have_volatile)
Call it CheckDomainConstraints or something instead? IMO it's confusing
the have it not return what it's called.
Also, it'd make it more self-contained and thus safer to initialise have_volatile to false.
+ if (typentry->domainData != NULL)
+ {
+ if (have_volatile)
+ {
+ foreach_node(DomainConstraintState, constrstate,
+ typentry->domainData->constraints)
+ {
+ if (constrstate->constrainttype == DOM_CONSTRAINT_CHECK &&
+ contain_volatile_functions((Node *) constrstate->check_expr))
+ {
+ *have_volatile = true;
+ break;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ return false;
+}
Could simplify the code by doing an early return if domainData == NULL?
(same with have_volatile below)
+ /*
+ * If the domain has volatile constraints, we must do a table rewrite
+ * since the constraint result could differ per row and cannot be
+ * evaluated once and cached as a missing value.
+ */
+ if (has_volatile)
+ {
+ Assert(has_domain_constraints);
+ tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
+ }
I'm not sure. But seems to me this makes the pre-existing guard for virtual columns
redundant?
I mean this code on line 7633:
if (colDef->generated != ATTRIBUTE_GENERATED_VIRTUAL)
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
+-- test fast default over domains with constraints
+CREATE DOMAIN domain5 AS int CHECK(value > 10) DEFAULT 8;
+CREATE DOMAIN domain6 as int CHECK(value > 10) DEFAULT random(min=>11, max=>100);
+CREATE DOMAIN domain7 as int CHECK((value + random(min=>11::int, max=>11)) > 12);
+CREATE DOMAIN domain8 as int NOT NULL;
Would be nice to test domains with both volatile and non-volatile checks.
Also, perhaps virtual generated columns could use a test?
/Viktor Holmberg
