| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: ALTER EXTENSION SET SCHEMA versus dependent types |
| Date: | 2024-05-08 23:42:18 |
| Message-ID: | 947552.1715211738@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Nathan Bossart <nathandbossart(at)gmail(dot)com> writes:
> Looks reasonable to me. The added test coverage seems particularly
> valuable. If I really wanted to nitpick, I might complain about the three
> consecutive Boolean parameters for AlterTypeNamespaceInternal(), which
> makes lines like
> + AlterTypeNamespaceInternal(arrayOid, nspOid, true, false, true,
> + objsMoved);
> difficult to interpret. But that's not necessarily the fault of this patch
> and probably needn't block it.
I considered merging ignoreDependent and errorOnTableType into a
single 3-valued enum, but didn't think it was worth the trouble
given the very small number of callers; also it wasn't quite clear
how to map that to AlterTypeNamespace_oid's API. Perhaps a little
more thought is appropriate though.
One positive reason for increasing the number of parameters is that
that will be a clear API break for any outside callers, if there
are any. If I just replace a bool with an enum, such callers might
or might not get any indication that they need to take a fresh
look.
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nathan Bossart | 2024-05-08 23:54:59 | Re: ALTER EXTENSION SET SCHEMA versus dependent types |
| Previous Message | Nathan Bossart | 2024-05-08 23:33:20 | Re: ALTER EXTENSION SET SCHEMA versus dependent types |