Cascaded Column Drop
When a cascaded column drop is removing the last column, drop the table
instead. Regression tests via domains.
Reported by Tim Knowles.
--
Rod Taylor
Attachments:
lastcol_drop.patchtext/plain; charset=ISO-8859-1; name=lastcol_drop.patchDownload
? GNUmakefile
? config.log
? config.status
? contrib/spi/.deps
? src/Makefile.global
? src/backend/postgres
? src/backend/access/common/.deps
? src/backend/access/gist/.deps
? src/backend/access/hash/.deps
? src/backend/access/heap/.deps
? src/backend/access/index/.deps
? src/backend/access/nbtree/.deps
? src/backend/access/rtree/.deps
? src/backend/access/transam/.deps
? src/backend/bootstrap/.deps
? src/backend/catalog/.deps
? src/backend/catalog/postgres.bki
? src/backend/catalog/postgres.description
? src/backend/commands/.deps
? src/backend/executor/.deps
? src/backend/lib/.deps
? src/backend/libpq/.deps
? src/backend/main/.deps
? src/backend/nodes/.deps
? src/backend/optimizer/geqo/.deps
? src/backend/optimizer/path/.deps
? src/backend/optimizer/plan/.deps
? src/backend/optimizer/prep/.deps
? src/backend/optimizer/util/.deps
? src/backend/parser/.deps
? src/backend/port/.deps
? src/backend/postmaster/.deps
? src/backend/regex/.deps
? src/backend/rewrite/.deps
? src/backend/storage/buffer/.deps
? src/backend/storage/file/.deps
? src/backend/storage/freespace/.deps
? src/backend/storage/ipc/.deps
? src/backend/storage/large_object/.deps
? src/backend/storage/lmgr/.deps
? src/backend/storage/page/.deps
? src/backend/storage/smgr/.deps
? src/backend/tcop/.deps
? src/backend/utils/.deps
? src/backend/utils/adt/.deps
? src/backend/utils/cache/.deps
? src/backend/utils/error/.deps
? src/backend/utils/fmgr/.deps
? src/backend/utils/hash/.deps
? src/backend/utils/init/.deps
? src/backend/utils/mb/.deps
? src/backend/utils/mb/conversion_procs/conversion_create.sql
? src/backend/utils/mb/conversion_procs/ascii_and_mic/.deps
? src/backend/utils/mb/conversion_procs/ascii_and_mic/libascii_and_mic.so.0
? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/.deps
? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/libcyrillic_and_mic.so.0
? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/.deps
? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/libeuc_cn_and_mic.so.0
? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/.deps
? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/libeuc_jp_and_sjis.so.0
? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/.deps
? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/libeuc_kr_and_mic.so.0
? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/.deps
? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/libeuc_tw_and_big5.so.0
? src/backend/utils/mb/conversion_procs/latin2_and_win1250/.deps
? src/backend/utils/mb/conversion_procs/latin2_and_win1250/liblatin2_and_win1250.so.0
? src/backend/utils/mb/conversion_procs/latin_and_mic/.deps
? src/backend/utils/mb/conversion_procs/latin_and_mic/liblatin_and_mic.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_ascii/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_ascii/libutf8_and_ascii.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_big5/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_big5/libutf8_and_big5.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/libutf8_and_cyrillic.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/libutf8_and_euc_cn.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_jp/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_jp/libutf8_and_euc_jp.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_kr/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_kr/libutf8_and_euc_kr.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_tw/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_tw/libutf8_and_euc_tw.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_gb18030/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_gb18030/libutf8_and_gb18030.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_gbk/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_gbk/libutf8_and_gbk.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859/libutf8_and_iso8859.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/libutf8_and_iso8859_1.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_johab/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_johab/libutf8_and_johab.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_sjis/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_sjis/libutf8_and_sjis.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_tcvn/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_tcvn/libutf8_and_tcvn.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_uhc/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_uhc/libutf8_and_uhc.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_win1250/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_win1250/libutf8_and_win1250.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_win1256/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_win1256/libutf8_and_win1256.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_win874/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_win874/libutf8_and_win874.so.0
? src/backend/utils/misc/.deps
? src/backend/utils/mmgr/.deps
? src/backend/utils/sort/.deps
? src/backend/utils/time/.deps
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_config/pg_config
? src/bin/pg_controldata/.deps
? src/bin/pg_controldata/pg_controldata
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/.deps
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_dump/pg_restore
? src/bin/pg_encoding/.deps
? src/bin/pg_encoding/pg_encoding
? src/bin/pg_id/.deps
? src/bin/pg_id/pg_id
? src/bin/pg_resetxlog/.deps
? src/bin/pg_resetxlog/pg_resetxlog
? src/bin/psql/.deps
? src/bin/psql/psql
? src/bin/scripts/createlang
? src/include/pg_config.h
? src/include/stamp-h
? src/interfaces/ecpg/lib/.deps
? src/interfaces/ecpg/lib/libecpg.so.3
? src/interfaces/ecpg/preproc/.deps
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpq/.deps
? src/interfaces/libpq/libpq.so.2
? src/pl/plperl/.deps
? src/pl/plperl/SPI.c
? src/pl/plperl/libplperl.so.0
? src/pl/plpgsql/src/.deps
? src/pl/plpgsql/src/libplpgsql.so.1
? src/test/regress/.deps
? src/test/regress/log
? src/test/regress/pg_regress
? src/test/regress/regression.diffs
? src/test/regress/regression.out
? src/test/regress/results
? src/test/regress/tmp_check
? src/test/regress/expected/constraints.out
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/misc.out
? src/test/regress/sql/.domain.sql.swp
? src/test/regress/sql/constraints.sql
? src/test/regress/sql/copy.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/misc.sql
Index: src/backend/catalog/dependency.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/catalog/dependency.c,v
retrieving revision 1.12
diff -c -r1.12 dependency.c
*** src/backend/catalog/dependency.c 2002/09/22 00:37:09 1.12
--- src/backend/catalog/dependency.c 2002/09/27 00:56:27
***************
*** 631,639 ****
}
else
{
if (object->objectSubId != 0)
! RemoveAttributeById(object->objectId,
! object->objectSubId);
else
heap_drop_with_catalog(object->objectId);
}
--- 631,675 ----
}
else
{
+
if (object->objectSubId != 0)
! {
! Relation rel;
! TupleDesc tupleDesc;
! bool success = false;
! int n;
!
! /*
! * Make sure there will be at least one user column left in the
! * relation after we drop this one. Zero-length tuples tend to
! * confuse us.
! */
! rel = heap_open(object->objectId, AccessExclusiveLock);
!
! tupleDesc = RelationGetDescr(rel);
!
! for (n = 1; n <= tupleDesc->natts; n++)
! {
! Form_pg_attribute attribute = tupleDesc->attrs[n - 1];
!
! if (!attribute->attisdropped && n != object->objectSubId)
! {
! success = true;
! break;
! }
! }
!
! heap_close(rel, NoLock);
!
! /* More than one */
! if (success)
! RemoveAttributeById(object->objectId,
! object->objectSubId);
! /* Last attribute, drop the table instead */
! else
! heap_drop_with_catalog(object->objectId);
!
! }
else
heap_drop_with_catalog(object->objectId);
}
Index: src/test/regress/expected/domain.out
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/test/regress/expected/domain.out,v
retrieving revision 1.11
diff -c -r1.11 domain.out
*** src/test/regress/expected/domain.out 2002/09/20 03:52:50 1.11
--- src/test/regress/expected/domain.out 2002/09/27 00:56:48
***************
*** 187,189 ****
--- 187,198 ----
drop domain ddef3 restrict;
drop domain ddef4 restrict;
drop domain ddef5 restrict;
+ --
+ -- Ensure we drop the table when it's the last column
+ --
+ CREATE DOMAIN domlastcol as integer;
+ CREATE TABLE lastcol (col1 domlastcol);
+ DROP DOMAIN domlastcol CASCADE;
+ NOTICE: Drop cascades to table lastcol column col1
+ SELECT * FROM lastcol;
+ ERROR: Relation "lastcol" does not exist
Index: src/test/regress/sql/domain.sql
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/test/regress/sql/domain.sql,v
retrieving revision 1.8
diff -c -r1.8 domain.sql
*** src/test/regress/sql/domain.sql 2002/09/20 03:52:50 1.8
--- src/test/regress/sql/domain.sql 2002/09/27 00:56:49
***************
*** 154,156 ****
--- 154,164 ----
drop domain ddef3 restrict;
drop domain ddef4 restrict;
drop domain ddef5 restrict;
+
+ --
+ -- Ensure we drop the table when it's the last column
+ --
+ CREATE DOMAIN domlastcol as integer;
+ CREATE TABLE lastcol (col1 domlastcol);
+ DROP DOMAIN domlastcol CASCADE;
+ SELECT * FROM lastcol;
Rod Taylor <rbt@rbt.ca> writes:
When a cascaded column drop is removing the last column, drop the table
instead. Regression tests via domains.
Is that a good idea, or should we refuse the drop entirely? A table
drop zaps a lot more stuff than a column drop.
What I was actually wondering about after reading Tim's report was
whether we could support zero-column tables, which would eliminate the
need for the special case altogether. I have not looked to see how
extensive are the places that assume tuples have > 0 columns ...
regards, tom lane
Tom Lane wrote:
Rod Taylor <rbt@rbt.ca> writes:
When a cascaded column drop is removing the last column, drop the table
instead. Regression tests via domains.Is that a good idea, or should we refuse the drop entirely? A table
drop zaps a lot more stuff than a column drop.
I think we should refuse the drop. It is just too strange. You can
suggest if they want the column dropped, just drop the table.
What I was actually wondering about after reading Tim's report was
whether we could support zero-column tables, which would eliminate the
need for the special case altogether. I have not looked to see how
extensive are the places that assume tuples have > 0 columns ...
Zero-width tables do sound interesting. Is it somehow non-relational?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian wrote:
Tom Lane wrote:
What I was actually wondering about after reading Tim's report was
whether we could support zero-column tables, which would eliminate the
need for the special case altogether. I have not looked to see how
extensive are the places that assume tuples have > 0 columns ...Zero-width tables do sound interesting. Is it somehow non-relational?
I can see a zero column table as a transition state maybe, but what else would
they be useful for?
Joe
Joe Conway wrote:
Bruce Momjian wrote:
Tom Lane wrote:
What I was actually wondering about after reading Tim's report was
whether we could support zero-column tables, which would eliminate the
need for the special case altogether. I have not looked to see how
extensive are the places that assume tuples have > 0 columns ...Zero-width tables do sound interesting. Is it somehow non-relational?
I can see a zero column table as a transition state maybe, but what else would
they be useful for?
It's the /dev/null of the SQL world, but I'm not sure what that means. :-)
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian dijo:
Tom Lane wrote:
Rod Taylor <rbt@rbt.ca> writes:
When a cascaded column drop is removing the last column, drop the table
instead. Regression tests via domains.Is that a good idea, or should we refuse the drop entirely? A table
drop zaps a lot more stuff than a column drop.I think we should refuse the drop. It is just too strange. You can
suggest if they want the column dropped, just drop the table.
Yeah... you can't have triggers, you can't have constraints. Hey, you
can create a view using it, and possibly you can inherit the table...
but what's that good for?
But think about the inheritance case again: suppose
create table p (f1 int);
create table c (f2 int) inherits (p);
Now you just change your mind and want to drop p but not c. You can't
do it because f1 is the last column on it, and c inherits it. So a way
to drop the last column inherited (thus freeing the dependency on p)
makes c independent, and you can drop p.
But note that this drop of p is not just drop-cascade: the inheritance
tree has to get out of the dependency info first (it's not drop-restrict
either, is it?)
--
Alvaro Herrera (<alvherre[a]atentus.com>)
"La espina, desde que nace, ya pincha" (Proverbio africano)
Bruce Momjian <pgman@candle.pha.pa.us> writes:
Tom Lane wrote:
What I was actually wondering about after reading Tim's report was
whether we could support zero-column tables, which would eliminate the
need for the special case altogether. I have not looked to see how
extensive are the places that assume tuples have > 0 columns ...
Zero-width tables do sound interesting. Is it somehow non-relational?
Dunno. I wasn't really thinking that zero-width tables are all that
useful by themselves, but it does seem natural that you should be able
to redefine a column by
ALTER TABLE mytab DROP COLUMN foo;
ALTER TABLE mytab ADD COLUMN foo ...;
even if foo is the only column in mytab. Right now we reject that.
regards, tom lane
Is that a good idea, or should we refuse the drop entirely? A table
drop zaps a lot more stuff than a column drop.I think we should refuse the drop. It is just too strange. You can
suggest if they want the column dropped, just drop the table.
Leaving a zero-width table would be best, even if its not so useful. I
don't like rejecting a CASCADE as it kinda defeats the purpose of having
CASCADE.
The patch may not work right in complex cases anyway. It seems the
switch to remove the table should be earlier.
--
Rod Taylor
Alvaro Herrera <alvherre@atentus.com> writes:
But think about the inheritance case again: suppose
create table p (f1 int);
create table c (f2 int) inherits (p);
Now you just change your mind and want to drop p but not c. You can't
do it because f1 is the last column on it, and c inherits it. So a way
to drop the last column inherited (thus freeing the dependency on p)
makes c independent, and you can drop p.
Hmm, no I don't think so. Parent-to-child dependence is a property of
the two tables, not of their columns, and should not go away just
because you reduce the parent to zero columns. I would expect that if
I dropped p.f1 (assuming this were allowed) and then added p.g1, that
c would also now have c.g1. So the parent/child relationship outlives
any specific column ... IMHO anyway.
regards, tom lane
Rod Taylor <rbt@rbt.ca> writes:
Leaving a zero-width table would be best, even if its not so useful. I
don't like rejecting a CASCADE as it kinda defeats the purpose of having
CASCADE.
I did something about this --- as of CVS tip, you can do
regression=# create table foo (f1 int);
CREATE TABLE
regression=# alter table foo drop column f1;
ALTER TABLE
regression=# select * from foo;
--
(0 rows)
I fixed the places that were exposed by the regression tests as not
coping with zero-column tables, but it is likely that there are some
more places that will give odd errors with such a table. Feel free
to beat on it.
psql seems to have some minor issues with a zero-column select.
You can do this:
regression=# insert into foo default values;
INSERT 720976 1
regression=# select * from foo;
--
(1 row)
regression=# insert into foo default values;
INSERT 720977 1
regression=# select * from foo;
--
(2 rows)
regression=#
Seems like nothing's being printed for an empty row.
regards, tom lane
On Sat, 2002-09-28 at 16:38, Tom Lane wrote:
Rod Taylor <rbt@rbt.ca> writes:
Leaving a zero-width table would be best, even if its not so useful. I
don't like rejecting a CASCADE as it kinda defeats the purpose of having
CASCADE.I did something about this --- as of CVS tip, you can do
regression=# create table foo (f1 int);
CREATE TABLE
regression=# alter table foo drop column f1;
ALTER TABLE
regression=# select * from foo;
Which of course would dump as 'create table foo ();'.
I don't think relcache would like a table without any columns, which is
why the above is rejected.
Anyway, should pg_dump ignore the table entirely? Or do we try to allow
create table () without any attributes?
--
Rod Taylor
Rod Taylor <rbt@rbt.ca> writes:
I did something about this --- as of CVS tip, you can do
regression=# create table foo (f1 int);
CREATE TABLE
regression=# alter table foo drop column f1;
ALTER TABLE
regression=# select * from foo;
Which of course would dump as 'create table foo ();'.
True. I didn't say that everything would be happy with it ;-). I think
that a zero-column table is only useful as a transient state, and so I'm
happy as long as the backend doesn't core dump.
I don't think relcache would like a table without any columns, which is
why the above is rejected.
Relcache doesn't seem to have a problem with it.
Anyway, should pg_dump ignore the table entirely? Or do we try to allow
create table () without any attributes?
I feel no strong need to do either. But it likely would only take
removal of this error check:
regression=# create table foo ();
ERROR: DefineRelation: please inherit from a relation or define an attribute
at least as far as the backend goes.
regards, tom lane
regression=# create table foo ();
ERROR: DefineRelation: please inherit from a relation or define an attributeat least as far as the backend goes.
Found in relcache.c earlier:
AssertArg(natts > 0);
Didn't look too hard to see what it protects, because it's more effort
than it's worth.
--
Rod Taylor