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 | Resend email |
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 |
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 |