Re: [HACKERS] ALTER COLUMN TYPE vs. domain constraints

[Tom Lane]

Michael Paquier  writes:
> On Fri, Oct 27, 2017 at 11:15 AM, Tom Lane  wrote:
>> We could consider back-patching the attached to cover this, but
>> I'm not entirely sure it's worth the trouble, because I haven't
>> thought of any non-silly use-cases in the absence of domains
>> over composite.  Comments?

> There are no real complaints about the current behavior, aren't there?
> So only patching HEAD seems enough to me.

Yeah, we can leave it till someone does complain.

> You have added a comment on the constraint to make sure that it
> remains up on basically this ALTER TYPE. Querying pg_obj_description
> would make sure that the comment on the constraint is still here.

Done.

> +RebuildDomainConstraintComment(AlteredTableInfo *tab, int pass, Oid objid,
> +  List *domname, char *conname)
> There is much duplication with RebuildConstraintComment. Why not
> grouping both under say RebuildObjectComment()?

True.  I'd originally expected the code to differ more, but we can
merge these easily enough.

> I would think about
> having cmd->objtype and cmd->object passed as arguments, and then
> remove rel and domname from the existing arguments.

Doing it like that requires the callers to do work (to prepare the object
ID data structure) that's wasted if there's no comment, which most often
there wouldn't be, I suspect.  Seems better to just pass the info the
caller does have and let this function do the rest.

Pushed, thanks for reviewing!

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] ALTER COLUMN TYPE vs. domain constraints

[Michael Paquier]

On Fri, Oct 27, 2017 at 11:15 AM, Tom Lane  wrote:
> We could consider back-patching the attached to cover this, but
> I'm not entirely sure it's worth the trouble, because I haven't
> thought of any non-silly use-cases in the absence of domains
> over composite.  Comments?

There are no real complaints about the current behavior, aren't there?
So only patching HEAD seems enough to me.

+comment on constraint c1 on domain dcomptype is 'random commentary';
[...]
+alter type comptype alter attribute r type bigint;
You have added a comment on the constraint to make sure that it
remains up on basically this ALTER TYPE. Querying pg_obj_description
would make sure that the comment on the constraint is still here.

+static void
+RebuildDomainConstraintComment(AlteredTableInfo *tab, int pass, Oid objid,
+  List *domname, char *conname)
There is much duplication with RebuildConstraintComment. Why not
grouping both under say RebuildObjectComment()? I would think about
having cmd->objtype and cmd->object passed as arguments, and then
remove rel and domname from the existing arguments.

[nit]
foreach(lcmd, subcmds)
-   ATExecCmd(wqueue, tab, rel, (AlterTableCmd *)
lfirst(lcmd), lockmode);
+   ATExecCmd(wqueue, tab, rel,
+ castNode(AlterTableCmd, lfirst(lcmd)),
+ lockmode);
This does not really belong to this patch.. No objections to group things.
[/nit]
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers