Обсуждение: ALTER TYPE recursion to typed tables
I'm working on propagating ALTER TYPE commands to typed tables. This is currently prohibited. For example, take these regression test cases: CREATE TYPE test_type2 AS (a int, b text); CREATE TABLE test_tbl2 OF test_type2; ALTER TYPE test_type2 ADD ATTRIBUTE c text; -- fails ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar; -- fails ALTER TYPE test_type2 DROP ATTRIBUTE b; -- fails ALTER TYPE test_type2 RENAME ATTRIBUTE b TO bb; -- fails The actual implementation isn't very difficult, because the ALTER TABLE code already knows everything about recursion. Now I'm wondering what kind of syntax should be used to control this. I think you don't want to automatically propagate such innocent looking operations to tables in a potentially data-destroying manner. The natural idea would be RESTRICT/CASCADE. This is currently only associated with DROP operations, but I suppose ADD/ALTER/RENAME ATTRIBUTE x ... CASCADE doesn't sound too odd. Comments, other ideas?
On Tue, Nov 2, 2010 at 9:15 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > I'm working on propagating ALTER TYPE commands to typed tables. This is > currently prohibited. For example, take these regression test cases: > > CREATE TYPE test_type2 AS (a int, b text); > CREATE TABLE test_tbl2 OF test_type2; > ALTER TYPE test_type2 ADD ATTRIBUTE c text; -- fails > ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar; -- fails > ALTER TYPE test_type2 DROP ATTRIBUTE b; -- fails > ALTER TYPE test_type2 RENAME ATTRIBUTE b TO bb; -- fails > > The actual implementation isn't very difficult, because the ALTER TABLE > code already knows everything about recursion. > > Now I'm wondering what kind of syntax should be used to control this. I > think you don't want to automatically propagate such innocent looking > operations to tables in a potentially data-destroying manner. The > natural idea would be RESTRICT/CASCADE. This is currently only > associated with DROP operations, but I suppose ADD/ALTER/RENAME > ATTRIBUTE x ... CASCADE doesn't sound too odd. > > Comments, other ideas? That seems reasonable. What do you plan to do about this case? CREATE TYPE test_type AS (a int, b text); CREATE TABLE test_tbl (x test_type); ALTER TYPE test_type ADD ATTRIBUTE c text; -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On tis, 2010-11-02 at 10:54 -0700, Robert Haas wrote: > What do you plan to do about this case? > > CREATE TYPE test_type AS (a int, b text); > CREATE TABLE test_tbl (x test_type); > ALTER TYPE test_type ADD ATTRIBUTE c text; This is currently prohibited, and I'm not planning to do anything about that.
On Nov 2, 2010, at 11:17 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On tis, 2010-11-02 at 10:54 -0700, Robert Haas wrote: >> What do you plan to do about this case? >> >> CREATE TYPE test_type AS (a int, b text); >> CREATE TABLE test_tbl (x test_type); >> ALTER TYPE test_type ADD ATTRIBUTE c text; > > This is currently prohibited, and I'm not planning to do anything about > that. OK. ...Robert
Here is the patch that adds [RESTRICT|CASCADE] to ALTER TYPE ... ADD/ALTER/DROP/RENAME ATTRIBUTE, so that recurses to typed tables.
Вложения
Peter Eisentraut <peter_e@gmx.net> writes: > Here is the patch that adds [RESTRICT|CASCADE] to ALTER TYPE ... > ADD/ALTER/DROP/RENAME ATTRIBUTE, so that recurses to typed tables. And here's my commitfest review of it: - patch applies cleanly- adds regression tests- passes them- is useful and needed, and something we don't already have- don'tgenerate warnings (or I missed them) :) Code wise, though, I wonder about the name of the "recursing" parameter of the renameatt_internal function is src/backend/commands/tablecmds.c, which seems to only get used to detect erroneous attempt at renaming the table column directly. Maybe it's only me not used enough to PostgreSQL code yet, but here it distract the code reader. Having another parameter called "recurse" is not helping, too, but I don't see this one needs to be changed. I'm not sure what a good name would be here, alter_type_cascade is an example that comes to mind, on the verbose side. As I think the issue is to be decided by a commiter, I will go and mark this patch as ready for commiter! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On ons, 2010-11-17 at 21:05 +0100, Dimitri Fontaine wrote: > Code wise, though, I wonder about the name of the "recursing" > parameter of the renameatt_internal function is > src/backend/commands/tablecmds.c, > which seems to only get used to detect erroneous attempt at renaming > the table column directly. Maybe it's only me not used enough to > PostgreSQL code yet, but here it distract the code reader. Having > another parameter called "recurse" is not helping, too, but I don't > see this one needs to be changed. This parameter has only minimal use in the renameatt case, but the same terminology is used throughout the ALTER TABLE code, so I think it's wise to keep it.