Re: using index or check in ALTER TABLE SET NOT NULL

From: Sergei Kornilov <sk(at)zsrv(dot)org>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>, Ildar Musin <i(dot)musin(at)postgrespro(dot)ru>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: using index or check in ALTER TABLE SET NOT NULL
Date: 2019-03-12 20:27:47
Message-ID: 694691552422467@myt1-06117f29c1ea.qloud-c.yandex.net
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hello

> Dispatches from the department of grammatical nitpicking...

Thank you!

> + entire table, however if a valid <literal>CHECK</literal> constraint is
>
> I think this should be:
>
> entire table; however, if...
>
> + * are set NOT NULL, however, if we can find a constraint which proves
>
> similarly here

Changed

> + ereport(DEBUG1,
> + (errmsg("verifying table \"%s\" NOT NULL constraint "
> + "on %s attribute by existed constraints",
> + RelationGetRelationName(rel), NameStr(attr->attname))));
>
> Ugh, that doesn't read well at all. How about:
>
> existing constraints on column "%s"."%s" are sufficient to prove that
> it does not contain nulls

Changed

> - * in implicit-AND form.
> + * in implicitly-AND form, must only contain immutable clauses
> + * and all Vars must be varno=1.
>
> I think you should leave the existing sentence alone (implicit-AND is
> correct, implicitly-AND is not) and add a new sentence that says the
> stuff you want to add.

Ok

> + * "Existing constraints" include its check constraints and optional
> + * caller-provided existConstraint list. existConstraint list is modified
> + * during ConstraintImpliedByRelConstraint call and would represent all
> + * assumed conditions. testConstraint describes the constraint to validate.
> + * Both existConstraint and testConstraint must be in implicitly-AND form,
> + * must only contain immutable clauses and all Vars must be varno=1.
>
> I think that it might be better to copy the list rather than to have
> the comment note that it gets mutated, but regardless the grammar
> needs improvement here.

Agreed, in attached new version I copy the list and do not modify parameter.

> I object to removing this.

Okay, I revert this David's change

regards, Sergei

Attachment Content-Type Size
0001_alter_table_set_not_null_v12.patch text/x-diff 12.6 KB
0002_alter_table_set_not_null_v12_debuglevel_tests.patch text/x-diff 5.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2019-03-12 21:08:19 Re: Offline enabling/disabling of data checksums
Previous Message Jesper Pedersen 2019-03-12 20:24:01 Re: pg_upgrade: Pass -j down to vacuumdb