| From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
|---|---|
| To: | Viktor Holmberg <v(at)viktorh(dot)net>, jian he <jian(dot)universality(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: support fast default for domain with constraints |
| Date: | 2026-03-11 19:50:23 |
| Message-ID: | 31d12caf-17d8-4ab7-af58-ec3c5a310a92@dunslane.net |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-03-11 We 6:34 AM, Viktor Holmberg wrote:
> 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)
I think it's cleaner just to modify the existing function with an extra
parameter, which the existing callers will pass as NULL.
>
>
>
>
> Would be nice to test domains with both volatile and non-volatile checks.
> Also, perhaps virtual generated columns could use a test?
>
>
Also added some tests.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
| Attachment | Content-Type | Size |
|---|---|---|
| v11-0001-Extend-DomainHasConstraints-to-optionally-check-.patch | text/x-patch | 6.5 KB |
| v11-0002-Enable-fast-default-for-domains-with-non-volatil.patch | text/x-patch | 16.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-03-11 19:52:35 | Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc? |
| Previous Message | Laurenz Albe | 2026-03-11 19:44:36 | Re: [PATCH] libpq: try all addresses for a host before moving to next on target_session_attrs mismatch |