Обсуждение: ALTER TABLE ADD COLUMN column SERIAL -- unexpected results
Running: ALTER TABLE table ADD COLUMN column SERIAL; Defines a column as int4 but does not create the sequence or attempt to set the default value. Not a big deal, but I was surprised when the column values were null. -- Rod Taylor Your eyes are weary from staring at the CRT. You feel sleepy. Notice how restful it is to watch the cursor blink. Close your eyes. The opinions stated above are yours. You cannot imagine why you ever felt otherwise.
"Rod Taylor" <rbt@barchord.com> writes:
> Running:
> ALTER TABLE table ADD COLUMN column SERIAL;
> Defines a column as int4 but does not create the sequence or attempt
> to set the default value.
Yeah ... SERIAL is implemented as a hack in the parsing of CREATE
TABLE, but there's no corresponding hack in ALTER TABLE. A bug,
no doubt about it, but I don't much like the obvious fix of duplicating
the hack in two places. Isn't there a cleaner way to deal with this
"data type"?
regards, tom lane
> "Rod Taylor" <rbt@barchord.com> writes: > > Running: > > ALTER TABLE table ADD COLUMN column SERIAL; > > Defines a column as int4 but does not create the sequence or attempt > > to set the default value. > > Yeah ... SERIAL is implemented as a hack in the parsing of CREATE > TABLE, but there's no corresponding hack in ALTER TABLE. A bug, > no doubt about it, but I don't much like the obvious fix of duplicating > the hack in two places. Isn't there a cleaner way to deal with this > "data type"? Added to TODO. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Tom Lane wrote: > > "Rod Taylor" <rbt@barchord.com> writes: > > Running: > > ALTER TABLE table ADD COLUMN column SERIAL; > > Defines a column as int4 but does not create the sequence or attempt > > to set the default value. > > Yeah ... SERIAL is implemented as a hack in the parsing of CREATE > TABLE, but there's no corresponding hack in ALTER TABLE. A bug, > no doubt about it, but I don't much like the obvious fix of duplicating > the hack in two places. Isn't there a cleaner way to deal with this > "data type"? > *ALTER TABLE* isn't as easy as *CREATE TABLE*. It has another problem because it hasn't implemented *DEFAULT* yet. regards, Hiroshi Inoue
Christopher Kings-Lynne wrote: > > > *ALTER TABLE* isn't as easy as *CREATE TABLE*. > > It has another problem because it hasn't implemented > > *DEFAULT* yet. > > Just out of interest, is there a special reason it's difficult to implement > the DEFAULT feature of alter table add column? > Without *DEFAULT* we don't have to touch the table file at all. With *DEFAULT* we have to fill the new column with the *DEFAULT* value for all existent rows. regards, Hiroshi Inoue
> *ALTER TABLE* isn't as easy as *CREATE TABLE*. > It has another problem because it hasn't implemented > *DEFAULT* yet. Just out of interest, is there a special reason it's difficult to implement the DEFAULT feature of alter table add column? Chris
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> Christopher Kings-Lynne wrote:
>> Just out of interest, is there a special reason it's difficult to implement
>> the DEFAULT feature of alter table add column?
> Without *DEFAULT* we don't have to touch the table file
> at all. With *DEFAULT* we have to fill the new column
> with the *DEFAULT* value for all existent rows.
Do we? We could simply declare by fiat that the behavior of ALTER ADD
COLUMN is to fill the new column with nulls. Let the user do an UPDATE
to fill the column with a default, if he wants to. After all, I'd not
expect that an ALTER that adds a DEFAULT spec to an existing column
would go through and replace existing NULL entries for me.
This is a little trickier if one wants to make a NOT NULL column,
however. Seems the standard technique for that could be
ALTER tab ADD COLUMN newcol without the not null spec;UPDATE tab SET newcol = something;ALTER tab ALTER COLUMN newcol
ADDCONSTRAINT NOT NULL;
where the last command would verify that the column contains no nulls
before setting the flag, just like ALTER TABLE ADD CONSTRAINT does now
(but I think we don't have a variant for NULL/NOT NULL constraints).
This is slightly ugly, maybe, but it sure beats not having the feature
at all. Besides, it seems to me there are cases where you don't really
*want* the DEFAULT value to be used to fill the column, but something
else (or even want NULLs). Why should the system force an update of
every row in the table with a value that might not be what the user
wants?
regards, tom lane
Tom Lane writes: > Besides, it seems to me there are cases where you don't really > *want* the DEFAULT value to be used to fill the column, but something > else (or even want NULLs). Then you could use ALTER TABLE t1 ADD COLUMN cn text; ALTER TABLE t1 ALTER COLUMN cn SET DEFAULT 'what you really wanted'; A subtle difference, but it's perfectly consistent. -- And it works already. -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter
Tom Lane wrote: > > Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > Christopher Kings-Lynne wrote: > >> Just out of interest, is there a special reason it's difficult to implement > >> the DEFAULT feature of alter table add column? > > > Without *DEFAULT* we don't have to touch the table file > > at all. With *DEFAULT* we have to fill the new column > > with the *DEFAULT* value for all existent rows. > > Do we? We could simply declare by fiat that the behavior of ALTER ADD > COLUMN is to fill the new column with nulls. Let the user do an UPDATE > to fill the column with a default, if he wants to. I don't like to fill the column of the existent rows but it seems to be the spec. > After all, I'd not > expect that an ALTER that adds a DEFAULT spec to an existing column > would go through and replace existing NULL entries for me. > > This is a little trickier if one wants to make a NOT NULL column, > however. Seems the standard technique for that could be > > ALTER tab ADD COLUMN newcol without the not null spec; > UPDATE tab SET newcol = something; > ALTER tab ALTER COLUMN newcol ADD CONSTRAINT NOT NULL; > Yes I love this also. regards, Hiroshi Inoue
> Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > Christopher Kings-Lynne wrote: > >> Just out of interest, is there a special reason it's difficult to implement > >> the DEFAULT feature of alter table add column? > > > Without *DEFAULT* we don't have to touch the table file > > at all. With *DEFAULT* we have to fill the new column > > with the *DEFAULT* value for all existent rows. > > Do we? We could simply declare by fiat that the behavior of ALTER ADD > COLUMN is to fill the new column with nulls. Let the user do an UPDATE > to fill the column with a default, if he wants to. After all, I'd not > expect that an ALTER that adds a DEFAULT spec to an existing column > would go through and replace existing NULL entries for me. > > This is a little trickier if one wants to make a NOT NULL column, > however. Seems the standard technique for that could be > > ALTER tab ADD COLUMN newcol without the not null spec; > UPDATE tab SET newcol = something; > ALTER tab ALTER COLUMN newcol ADD CONSTRAINT NOT NULL; > > where the last command would verify that the column contains no nulls > before setting the flag, just like ALTER TABLE ADD CONSTRAINT does now > (but I think we don't have a variant for NULL/NOT NULL constraints). > > This is slightly ugly, maybe, but it sure beats not having the feature > at all. Besides, it seems to me there are cases where you don't really > *want* the DEFAULT value to be used to fill the column, but something > else (or even want NULLs). Why should the system force an update of > every row in the table with a value that might not be what the user > wants? I am trying to find a way to get this information to users. I have modified command.c to output a different error message: test=> alter table x add column z int default 4; ERROR: Adding columns with defaults is not implemented because it is unclear whether existing rows should have theDEFAULT value or NULL. Add the column, then use ALTER TABLE SET DEFAULT. You may then use UPDATE to givea non-NULL value to existing rows. How does this sound? Peter, should I keep it for 7.3 so I don't mess up the translations in 7.2? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I am trying to find a way to get this information to users. I have
> modified command.c to output a different error message:
> test=> alter table x add column z int default 4;
> ERROR: Adding columns with defaults is not implemented because it
> is unclear whether existing rows should have the DEFAULT value
> or NULL. Add the column, then use ALTER TABLE SET DEFAULT.
> You may then use UPDATE to give a non-NULL value to existing rows.
Kindly put the error message back as it was.
It's not "unclear" what the command should do; SQL92 is perfectly
clear about it.
I would also remind you that we've got quite a few sets of error message
translations in place now. Gratuitous changes to message wording in the
last week of beta are *not* appropriate, because they break all the
translations.
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I am trying to find a way to get this information to users. I have > > modified command.c to output a different error message: I should have used different wording here. I meant I tested a modification to command.c. > > test=> alter table x add column z int default 4; > > ERROR: Adding columns with defaults is not implemented because it > > is unclear whether existing rows should have the DEFAULT value > > or NULL. Add the column, then use ALTER TABLE SET DEFAULT. > > You may then use UPDATE to give a non-NULL value to existing rows. > > Kindly put the error message back as it was. > > It's not "unclear" what the command should do; SQL92 is perfectly > clear about it. > > I would also remind you that we've got quite a few sets of error message > translations in place now. Gratuitous changes to message wording in the > last week of beta are *not* appropriate, because they break all the > translations. If you read a little further you would have seen: > How does this sound? Peter, should I keep it for 7.3 so I don't mess up > the translations in 7.2? I was not about to apply it. I need comments on how we should communicate this to the user. I have email from you from July saying: > > Without *DEFAULT* we don't have to touch the table file > > at all. With *DEFAULT* we have to fill the new column > > with the *DEFAULT* value for all existent rows. > > Do we? We could simply declare by fiat that the behavior of ALTER ADD > COLUMN is to fill the new column with nulls. Let the user do an UPDATE > to fill the column with a default, if he wants to. After all, I'd not > expect that an ALTER that adds a DEFAULT spec to an existing column > would go through and replace existing NULL entries for me. Then Hiroshi saying: > I don't like to fill the column of the existent rows but > it seems to be the spec. So, are we now all agreed that we have to fill in the existing rows with the default value? If so, I can document that in the TODO list and discard this patch. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > So, are we now all agreed that we have to fill in the existing rows with
> > the default value?
>
> I think there's no debate that that's what the spec says to do. There
> might be some discontent in the ranks about whether to follow SQL92's
> marching orders, however. Doubling the filesize of the table to apply
> changes that the user might not even want seems a heavy penalty.
>
> > If so, I can document that in the TODO list and
> > discard this patch.
>
> This is certainly a TODO or TOARGUEABOUT item, not something to be
> patched on short notice.
Agreed. I didn't think it was going into 7.2 anyway. I am just looking
to see if we will solve this with code or we will solve it with
documentation. If it was documentation, I was going to see if I could
come up with some wording.
However, because the spec is clear, seems we will have to do some
codework on this later. Added to TODO:
o ALTER TABLE ADD COLUMN column SET DEFAULT should fill existing rows with DEFAULT value or allow NULLs
inexisting rows
My guess is that we will have to do spec behavior by default, and add a
flag to allow NULLs in existing rows.
Yuck, but at least we have a direction.
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> So, are we now all agreed that we have to fill in the existing rows with
> the default value?
I think there's no debate that that's what the spec says to do. There
might be some discontent in the ranks about whether to follow SQL92's
marching orders, however. Doubling the filesize of the table to apply
changes that the user might not even want seems a heavy penalty.
> If so, I can document that in the TODO list and
> discard this patch.
This is certainly a TODO or TOARGUEABOUT item, not something to be
patched on short notice.
regards, tom lane
Bruce Momjian writes: > o ALTER TABLE ADD COLUMN column SET DEFAULT should fill existing > rows with DEFAULT value or allow NULLs in existing rows > > My guess is that we will have to do spec behavior by default, and add a > flag to allow NULLs in existing rows. May I point you to the already existing set of commands that do exactly what you want: ALTER TABLE t1 ADD COLUMN name type; ALTER TABLE t1 ALTER COLUMN name SET DEFAULT foo; There's no reason to muck around with the spec here. -- Peter Eisentraut peter_e@gmx.net
> Bruce Momjian writes: > > > o ALTER TABLE ADD COLUMN column SET DEFAULT should fill existing > > rows with DEFAULT value or allow NULLs in existing rows > > > > My guess is that we will have to do spec behavior by default, and add a > > flag to allow NULLs in existing rows. > > May I point you to the already existing set of commands that do exactly > what you want: > > ALTER TABLE t1 ADD COLUMN name type; > ALTER TABLE t1 ALTER COLUMN name SET DEFAULT foo; > > There's no reason to muck around with the spec here. OK, so we will implement the spec, and allow people to avoid the double-disk space by doing the steps manually and skipping the UPDATE. Sounds good to me. TODO updated. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026