Re: [HACKERS] [COMMITTERS] pgsql: Disallow changing/dropping default
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Tom Lane wrote: > >> Wasn't this patch rejected? > > > Anyway, what is your opinion on this? > > I thought we'd rejected it. I'm not sure that we'd completely agreed > what the best thing to do is, but what this patch actually does is to > silently remove the dependency link. That is, after > > create table t1 (f1 serial); > alter table t1 alter column f1 drop default; > > t1_f1_seq is still there, but now completely unconnected to t1. > That doesn't seem to me to satisfy the principle of least surprise. > It's certainly not what the TODO item says (reject the DROP DEFAULT). > I think we were considering the alternative of having the DROP DEFAULT > remove the sequence, which probably could be implemented painlessly > with a change in the way we set up the dependency links to start with. > > In any case I don't like this patch: int/bool confusion, use of elog > instead of ereport for a user-facing error message, failure to adhere to > style guidelines for that message, etc. (Although seeing that the error > message is unreachable code, maybe that doesn't matter ;-)) Aside from > the poor coding style, the whole idea of reaching into pg_depend to > remove a single dependency strikes me as a brute-force solution to > a problem that should have a more elegant answer. Agreed, patch reverted. Thanks for the analysis. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [COMMITTERS] pgsql: Disallow changing/dropping default expression
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Wasn't this patch rejected? > Anyway, what is your opinion on this? I thought we'd rejected it. I'm not sure that we'd completely agreed what the best thing to do is, but what this patch actually does is to silently remove the dependency link. That is, after create table t1 (f1 serial); alter table t1 alter column f1 drop default; t1_f1_seq is still there, but now completely unconnected to t1. That doesn't seem to me to satisfy the principle of least surprise. It's certainly not what the TODO item says (reject the DROP DEFAULT). I think we were considering the alternative of having the DROP DEFAULT remove the sequence, which probably could be implemented painlessly with a change in the way we set up the dependency links to start with. In any case I don't like this patch: int/bool confusion, use of elog instead of ereport for a user-facing error message, failure to adhere to style guidelines for that message, etc. (Although seeing that the error message is unreachable code, maybe that doesn't matter ;-)) Aside from the poor coding style, the whole idea of reaching into pg_depend to remove a single dependency strikes me as a brute-force solution to a problem that should have a more elegant answer. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [COMMITTERS] pgsql: Disallow changing/dropping default expression
Tom Lane wrote: > [EMAIL PROTECTED] (Bruce Momjian) writes: > > Disallow changing/dropping default expression of a SERIAL column > > Wasn't this patch rejected? Your summary is utterly wrong about what > it does, which makes me wonder whether you reviewed it at all. One > would also think that a change in user-visible behavior would merit > at least some documentation diffs. Well, it seemed it controlled whether dependency allowed you do drop a default sequence for a table. There is something I didn't like about the patch now that I look at it again --- it uses constants 0-2 when it should use defines or something clearer. I thought we had decided that we could not make SERIAL just a macro, and therefore we have to restrict how we allow modifications. If someone wants total control, they should create the DEFAULT manually, but SERIAL was going to be hard-wired. Anyway, what is your opinion on this? Yea, agreed on the documentation issue. That SERIAL distinction, if that is the direction we are going, should be documented. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
