Unix Technical Forum

Re: [PATCHES] Proposed patch for sequence-renaming problems

This is a discussion on Re: [PATCHES] Proposed patch for sequence-renaming problems within the pgsql Hackers forums, part of the PostgreSQL category; --> Tom Lane wrote: > > It is > > only the last one where recommending regclass helps, but is ...


Go Back   Unix Technical Forum > Database Server Software > PostgreSQL > pgsql Hackers

FAQ Members List Calendar Search Today's Posts Mark Forums Read
  #1 (permalink)  
Old 04-11-2008, 05:58 AM
Bruce Momjian
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Tom Lane wrote:
> > It is
> > only the last one where recommending regclass helps, but is it worth
> > improving sequence/schema renaming by exposing and recommending a
> > ::regclass syntax that will go away as soon as we fix this properly?

>
> Please explain what you think a "proper" fix is. I think this patch is
> a proper fix. I see no better alternative that we might implement
> later.
>
> The only other thing that's been discussed is the SQL2003 syntax
> NEXT VALUE FOR sequencename
> but this is in fact just syntactic sugar for something functionally
> equivalent to nextval('sequencename'::regclass). It cannot completely
> replace all uses of the nextval function, because only a constant table
> name can appear.
>
> Hmm ... given the proposed patch, it would indeed take only a few more
> lines in gram.y to support the NEXT VALUE FOR syntax ...


Just to follow up, I agree we can't totally replace all instances of
nextval() with regclass because regclass requires a constant string, but
I would like to have the regclass behavior with simple syntax and
require people who want "late binding" of the sequence name to use some
extra syntax, like ::text or something. This seems like the only way
sequence naming will be sustainable from release to release. Saying
"use ::regclass" over and over again, when 99% of users should be using
it for nextval in default clauses, is going to get very tiring.

The other question is whether we should be playing with this at all
during beta. Should we just disable ALTER SCHEMA RENAME and return to
this during 8.2? I am worried these side missions will delay our final
release of 8.1.

--
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

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #2 (permalink)  
Old 04-11-2008, 05:58 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Just to follow up, I agree we can't totally replace all instances of
> nextval() with regclass because regclass requires a constant string, but
> I would like to have the regclass behavior with simple syntax and
> require people who want "late binding" of the sequence name to use some
> extra syntax, like ::text or something.


That would require a considerably more invasive change, AFAICS: remove
the text-input version of nextval() and introduce an implicit coercion
from text to regclass to avoid breaking existing dumps. I'd prefer not
to mess with that during beta, because there'd be nontrivial risk of
breaking existing behaviors. Because the proposed patch just adds on
new functions and doesn't change the behavior of existing DEFAULT
clauses, I don't think it can break anything.

However, we could certainly add the NEXT VALUE FOR syntax if that will
satisfy your concern about syntax.

> The other question is whether we should be playing with this at all
> during beta. Should we just disable ALTER SCHEMA RENAME and return to
> this during 8.2? I am worried these side missions will delay our final
> release of 8.1.


I'm prepared to argue that this is a bug fix, not only for ALTER SCHEMA
RENAME but for some very long-standing problems with renaming of
sequences. As I said before, you are seriously mistaken to consider
that disabling ALTER SCHEMA RENAME would eliminate the problem.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #3 (permalink)  
Old 04-11-2008, 05:58 AM
Bruce Momjian
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Just to follow up, I agree we can't totally replace all instances of
> > nextval() with regclass because regclass requires a constant string, but
> > I would like to have the regclass behavior with simple syntax and
> > require people who want "late binding" of the sequence name to use some
> > extra syntax, like ::text or something.

>
> That would require a considerably more invasive change, AFAICS: remove
> the text-input version of nextval() and introduce an implicit coercion
> from text to regclass to avoid breaking existing dumps. I'd prefer not
> to mess with that during beta, because there'd be nontrivial risk of
> breaking existing behaviors. Because the proposed patch just adds on
> new functions and doesn't change the behavior of existing DEFAULT
> clauses, I don't think it can break anything.
>
> However, we could certainly add the NEXT VALUE FOR syntax if that will
> satisfy your concern about syntax.


I am personally fine with use ::regclass internally, especially for
SERIAL. It is documenting its use (and recommending it) that has me
concerned. We are placing additional burdens on users --- burdens that
will not exist in 8.2 when we have more time to fix it right.

Is it worth telling users to use ::regclass in their code for 8.1 just
to fix this, and then telling them in 8.2 it is not necessary to use
this?

> > The other question is whether we should be playing with this at all
> > during beta. Should we just disable ALTER SCHEMA RENAME and return to
> > this during 8.2? I am worried these side missions will delay our final
> > release of 8.1.

>
> I'm prepared to argue that this is a bug fix, not only for ALTER SCHEMA
> RENAME but for some very long-standing problems with renaming of
> sequences. As I said before, you are seriously mistaken to consider
> that disabling ALTER SCHEMA RENAME would eliminate the problem.


If it was that bad, we should have fixed it during development, not
during beta. The only reason it is getting attention now is because it
is triggered more by a new feature we are adding, a feature we can
easily remove.

I know we both don't want to open up the entire TODO list for fixing
during beta, especially fixing that isn't 100% complete and who's
user-visible behavior will change in the next major release.

Now, if we use ::regclass internally for just SERIAL, and don't document
its use for sequences (or at last minimize its visibility), or we add
NEXT VALUE FOR support and tell everyone to use that, that is fine with
me because it is probably the best way for users to use this in
defaults for all future releases.

Am I correct that NEXT VALUE FOR is behavior which will be
feature-complete and will be the recommended way to use sequences in
defaults in all future releases?

--
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

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #4 (permalink)  
Old 04-11-2008, 05:59 AM
Michael Paesold
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Tom Lane wrote:
> However, we could certainly add the NEXT VALUE FOR syntax if that will
> satisfy your concern about syntax.


Since the NEXT VALUE FOR syntax has a special meaning, would it be better to
support the oracle-style syntax sequence.nextval for now (and use the
::regclass for this)? I am not sure how easy that is considering
schema.sequence.nextval.

Just a thought.

Best Regards,
Michael Paesold


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #5 (permalink)  
Old 04-11-2008, 05:59 AM
Peter Eisentraut
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Am Mittwoch, 28. September 2005 18:10 schrieb Bruce Momjian:
> If it was that bad, we should have fixed it during development, not
> during beta. The only reason it is getting attention now is because it
> is triggered more by a new feature we are adding, a feature we can
> easily remove.


That was my thinking. The issue has probably been there since 7.3. I don't
think we need to shove in a solution now, especially when there is so much
disagreement about the behavior.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #6 (permalink)  
Old 04-11-2008, 06:02 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Peter Eisentraut <peter_e@gmx.net> writes:
> That was my thinking. The issue has probably been there since 7.3. I don't
> think we need to shove in a solution now, especially when there is so much
> disagreement about the behavior.


Well, we have a new issue that has made the problem much worse (ie ALTER
SCHEMA RENAME), and these problems are not going to get any easier to
solve later. I think we should agree on something and do it.

Ripping out ALTER SCHEMA RENAME is not a solution unless you have a path
to a solution later with more work. So far there has been nothing in
the way of "here is a proposal that will work but it'll take too much
time to implement for 8.1". Eventually we are going to have to settle
on one of the lesser evils, so why not now?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #7 (permalink)  
Old 04-11-2008, 06:02 AM
Bruce Momjian
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > That was my thinking. The issue has probably been there since 7.3. I don't
> > think we need to shove in a solution now, especially when there is so much
> > disagreement about the behavior.

>
> Well, we have a new issue that has made the problem much worse (ie ALTER
> SCHEMA RENAME), and these problems are not going to get any easier to
> solve later. I think we should agree on something and do it.
>
> Ripping out ALTER SCHEMA RENAME is not a solution unless you have a path
> to a solution later with more work. So far there has been nothing in
> the way of "here is a proposal that will work but it'll take too much
> time to implement for 8.1". Eventually we are going to have to settle
> on one of the lesser evils, so why not now?


Well, we are only giving ourselves a few weeks to solve this, and I
think a hack to make it work cleanly for users is better than supporting
two function names perpetually.

Remember the now, now(), CURRENT_TIMESTAMP issue of early binding. It
is still confusing to remember which is which, and doing it for
sequences new function names is confusing too.

--
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

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #8 (permalink)  
Old 04-11-2008, 06:02 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Ripping out ALTER SCHEMA RENAME is not a solution unless you have a path
>> to a solution later with more work.


> Well, we are only giving ourselves a few weeks to solve this, and I
> think a hack to make it work cleanly for users is better than supporting
> two function names perpetually.


Well, if you are dead set on having only one function name, then I think
the best solution is this:

* only one function, taking regclass

* add an implicit text-to-regclass coercion

With this, nextval('foo') is early binding and nextval('foo'::text) is
late binding, and existing dumps are going to continue to behave as late
binding unless changed manually.

The implicit coercion is a bit risky, but in practice these are likely
to be the only functions in the system that are declared to take
regclass, so the odds of the implicit coercion firing unexpectedly seem
low.

Does that sound like a workable compromise?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #9 (permalink)  
Old 04-11-2008, 06:02 AM
Bruce Momjian
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> Ripping out ALTER SCHEMA RENAME is not a solution unless you have a path
> >> to a solution later with more work.

>
> > Well, we are only giving ourselves a few weeks to solve this, and I
> > think a hack to make it work cleanly for users is better than supporting
> > two function names perpetually.

>
> Well, if you are dead set on having only one function name, then I think
> the best solution is this:
>
> * only one function, taking regclass
>
> * add an implicit text-to-regclass coercion
>
> With this, nextval('foo') is early binding and nextval('foo'::text) is
> late binding, and existing dumps are going to continue to behave as late
> binding unless changed manually.
>
> The implicit coercion is a bit risky, but in practice these are likely
> to be the only functions in the system that are declared to take
> regclass, so the odds of the implicit coercion firing unexpectedly seem
> low.
>
> Does that sound like a workable compromise?


Personally, I _love_ it. I hope others do as well. :-)

Let me explain why I thought two function names would be confusing. We
have been telling people to use nextval() since we added sequences in
6.4, and since 99% of people would want early binding (with
dependencies), I think making them all move to a new function name would
be a long-running education effort. If it can be avoided, that is
better.

I think the solution you propose is great because:

o it fixes SERIAL dependency
o it allows old dumps to load with no change in behavior
o it allows new nextval() calls to have early binding, unless
::text is used

In fact, the use of ::text is dump is the only thing that is _allowing_
this migration idea to work.

I think I can easily explain this issue in the release notes, and show
how users can update their schemas to the new behavior.

--
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

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #10 (permalink)  
Old 04-11-2008, 06:02 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Proposed patch for sequence-renaming problems

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Does that sound like a workable compromise?


> Personally, I _love_ it. I hope others do as well. :-)


OK, I'll work up a patch.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On
Forum Jump


All times are GMT. The time now is 01:35 AM.


Powered by vBulletin® Version 3.6.5
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
SEO by vBSEO 3.2.0
www.UnixAdminTalk.com