Thread: CREATE OR REPLACE FUNCTION
Hi guys, Here's a patch I put together. I thought the Oracle-style 'CREATE OR REPLACE FUNCTION' syntax might be useful to those doing lots of function creation/development. It is against the 7.1.3 source. I ran it through regress - all tests were fine. I also modified the regress to use the potential OR REPLACE syntax. This also worked. Gavin
Attachment
Gavin Sherry <swm@linuxworld.com.au> writes: > Here's a patch I put together. I thought the Oracle-style 'CREATE OR > REPLACE FUNCTION' syntax might be useful to those doing lots of function > creation/development. It is against the 7.1.3 source. Hmm. There are a couple of things that are a tad ugly about this patch --- you should be heap_update'ing the pg_proc entry, not doing a delete and insert --- but the main thing I don't like is that there's no checking to ensure that the function return type doesn't change. We can't allow that; it'd break stored views etc that use the function. It'd probably also be a good idea to insist that the replacer be the same as the original owner. (Possibly RemoveFunction does that for you in the patch as it stands, but it'll need an explicit test if you go the update route.) BTW, I've been assuming that when we got around to providing a capability like this, it'd be via an "ALTER FUNCTION" kind of statement. Does anyone have a strong feeling pro or con on whether "CREATE OR REPLACE" is a preferable approach? It doesn't seem to fit with the spirit of our other maintenance commands, but maybe we should just bow down before the Oracle and do it their way... regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Gavin Sherry <swm@linuxworld.com.au> writes: > > Here's a patch I put together. I thought the Oracle-style 'CREATE OR > > REPLACE FUNCTION' syntax might be useful to those doing lots of function > > creation/development. It is against the 7.1.3 source. > ... > BTW, I've been assuming that when we got around to providing a > capability like this, it'd be via an "ALTER FUNCTION" kind of > statement. Does anyone have a strong feeling pro or con on whether > "CREATE OR REPLACE" is a preferable approach? It doesn't seem to > fit with the spirit of our other maintenance commands, but maybe > we should just bow down before the Oracle and do it their way... ALTER FUNCTION seems strange to me, since the point of CREATE OR REPLACE FUNCTION really is to create the function. OR REPLACE is a convenience feature when loading PL/SQL (or PL/pgSQL) files into the database: you don't have to add a DROP FUNCTION command before each CREATE FUNCTION command, and you don't have to look at all the comments or warnings about dropping functions. We actually have an implementation of CREATE OR REPLACE FUNCTION at Zembu which I've never bothered rolling up and sending in. It simply drops the function first. Ian
> Gavin Sherry <swm@linuxworld.com.au> writes: > > Here's a patch I put together. I thought the Oracle-style 'CREATE OR > > REPLACE FUNCTION' syntax might be useful to those doing lots of function > > creation/development. It is against the 7.1.3 source. > > Hmm. There are a couple of things that are a tad ugly about this patch > --- you should be heap_update'ing the pg_proc entry, not doing a delete > and insert --- but the main thing I don't like is that there's no > checking to ensure that the function return type doesn't change. We > can't allow that; it'd break stored views etc that use the function. > > It'd probably also be a good idea to insist that the replacer be the > same as the original owner. (Possibly RemoveFunction does that for you > in the patch as it stands, but it'll need an explicit test if you go > the update route.) > > BTW, I've been assuming that when we got around to providing a > capability like this, it'd be via an "ALTER FUNCTION" kind of > statement. Does anyone have a strong feeling pro or con on whether > "CREATE OR REPLACE" is a preferable approach? It doesn't seem to > fit with the spirit of our other maintenance commands, but maybe > we should just bow down before the Oracle and do it their way... I assume it preserves the function's OID? That is something we have needed for a while because it would keep oid references to the function the same, or at least we should give people the option of keeping the oid. On the REPLACE/ALTER discussion, we have CREATE TABLE and ALTER TABLE. Seems ALTER FUNCTION is the way to go. Adding a REPLACE toplevel keyword for this feature just seems too harsh. -- 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, Pennsylvania 19026
On Thu, 20 Sep 2001, Tom Lane wrote: > Gavin Sherry <swm@linuxworld.com.au> writes: > > Here's a patch I put together. I thought the Oracle-style 'CREATE OR > > REPLACE FUNCTION' syntax might be useful to those doing lots of function > > creation/development. It is against the 7.1.3 source. > > Hmm. There are a couple of things that are a tad ugly about this patch > --- you should be heap_update'ing the pg_proc entry, not doing a delete Oops. Attached patch uses heap_update. I wasn't sure if I should be getting it from SearchSysCache() or with a direct heap scan. I used the first. > and insert --- but the main thing I don't like is that there's no > checking to ensure that the function return type doesn't change. We > can't allow that; it'd break stored views etc that use the function. This is in the attached patch. On this, the problem is that if a function is updated and has a different return type and the function is used by triggers, views etc then these mechanisms will be immediately corrupted. The same is true if a function whose return type remains the same generates incorrect data. Naturally this is the user's problem. But would it be useful to have a refcount in pg_proc which gets updated when triggers, views, etc are created/removed? If refcount == 0, the function can be updated (even with different return type), otherwise the user has to go to DROP FUNCTION. Seems like a fair bit of trouble involved. But may be useful. > > It'd probably also be a good idea to insist that the replacer be the > same as the original owner. (Possibly RemoveFunction does that for you > in the patch as it stands, but it'll need an explicit test if you go > the update route.) RemoveFunction() does that. I have included a check for this in the attached patch. > > BTW, I've been assuming that when we got around to providing a > capability like this, it'd be via an "ALTER FUNCTION" kind of > statement. Does anyone have a strong feeling pro or con on whether > "CREATE OR REPLACE" is a preferable approach? It doesn't seem to > fit with the spirit of our other maintenance commands, but maybe > we should just bow down before the Oracle and do it their way... I prefer the CREATE OR REPLACE syntax. It makes sense to want to replace a function even if you don't know if it exists. AS for bowing down to Oracle, I'm sure oracle users moving to Postgres would have no problem with syntax similiarities ;-) Either way, its a simple change to move it to an ALTER FUNCTION syntax. Gavin
Attachment
On Sat, 22 Sep 2001, Bruce Momjian wrote: [snip] > I assume it preserves the function's OID? That is something we have > needed for a while because it would keep oid references to the function > the same, or at least we should give people the option of keeping the > oid. Yes. The idea is that the oid is preserved. I have assumed that people would always want it to be if they were replacing the function. Would there be a time when someone wouldn't want to do this? It would be pretty simple to update the patch to handle a syntax which specified that a new oid should be generated. Gavin
> On Sat, 22 Sep 2001, Bruce Momjian wrote: > > [snip] > > > I assume it preserves the function's OID? That is something we have > > needed for a while because it would keep oid references to the function > > the same, or at least we should give people the option of keeping the > > oid. > > Yes. The idea is that the oid is preserved. I have assumed that people > would always want it to be if they were replacing the function. Would > there be a time when someone wouldn't want to do this? It would be pretty > simple to update the patch to handle a syntax which specified that a new > oid should be generated. Agreed. I think they always want the OID preserved. If they want a new one, they can drop/create. You have a good point about REPLACE allowing CREATE/ALTER depending on whether it already exists, but I see no reason to add this capability if it requires adding a new toplevel keyword. We already have tons of them. You could hack ALTER FUNCTION to create automatically if it doesn't exist, and throw a message to the user if you create it. However, if we decide we want REPLACE TABLE, etc then I think the REPLACE FUNCTION would be a good idea. People have asked for this capability and we have said, "Just drop the table first and ignore the error if it doesn't exist." The special case here is that the REPLACE/ALTER FUNCTION preserves the OID, which gives it a special capability not appropriate for the other objects. -- 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, Pennsylvania 19026
Bruce Momjian <pgman@candle.pha.pa.us> writes: > ... I see no reason to add this capability > if it requires adding a new toplevel keyword. That kind of annoys me too, but since the keyword can be made a TokenId, it won't create any conflicts. I think the argument of Oracle compatibility is probably a sufficient reason to do it with this syntax. > You could hack ALTER FUNCTION to create automatically if it > doesn't exist, That's considerably uglier than this alternative, wouldn't you say? We have no other ALTER commands that work that way. regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes: > I just hate to add another psql help item and manual page for a REPLACE > FUNCTION. It seems to asymetrical. Huh? Who said anything about a separate REPLACE FUNCTION command? It's just an option in CREATE. I don't think that I'd care for a separate REPLACE FUNCTION command, since that's just a peculiar spelling of ALTER FUNCTION, and historically we've used ALTER for that sort of thing. But that's not what's being proposed here anyway. regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > ... I see no reason to add this capability > > if it requires adding a new toplevel keyword. > > That kind of annoys me too, but since the keyword can be made a TokenId, > it won't create any conflicts. I think the argument of Oracle > compatibility is probably a sufficient reason to do it with this syntax. I just hate to add another psql help item and manual page for a REPLACE FUNCTION. It seems to asymetrical. Again, if we go all out and allow REPLACE for other items, it would be a big win. In fact, I can see a REPLACE TYPE/OPERATOR, but again ALTER would work for that too. > > You could hack ALTER FUNCTION to create automatically if it > > doesn't exist, > > That's considerably uglier than this alternative, wouldn't you say? > We have no other ALTER commands that work that way. Yes but that is assuming we want the ability to CREATE or ALTER in one command, which I am not sure we need. Just CREATE to create it and ALTER to replace it. We could use CARLETAETRE as C-R-E-A-T-E and A-L-T-E-R mixed. :-) Our stuff is different enough that I am not sure REPLACE has much validity. However, we could allow REPLACE FUNCTION to behave as ALTER FUNCTION and not advertize it so it would be used only for Oracle compatibility. OK, how about this. We do CREATE/ALTER FUNCTION and allow REPLACE to do CREATE/ALTER FUNCTION as needed. That way, we allow a specific way to alter a function that _will_ _fail_ if the function doesn't exist. And we add REPLACE TABLE to do that same where it drops and recreates the table if it already exists. People have asked for that in the past and we have told them to DROP and ignore the result. Seems this would be a nice win and be symetrical for all objects. And it gets us Oracle compatibility. (It would be nice for USER/INDEX/VIEW/SEQUENCE and the others too.) -- 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, Pennsylvania 19026
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I just hate to add another psql help item and manual page for a REPLACE > > FUNCTION. It seems to asymetrical. > > Huh? Who said anything about a separate REPLACE FUNCTION command? It's > just an option in CREATE. Oh, just an option to CREATE. Sorry. That sounds fine. And can we add this for other objects later? Sounds like a good plan. > I don't think that I'd care for a separate REPLACE FUNCTION command, > since that's just a peculiar spelling of ALTER FUNCTION, and > historically we've used ALTER for that sort of thing. But that's not > what's being proposed here anyway. Good. -- 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, Pennsylvania 19026
Patch applied. Thanks. > Hi guys, > > Here's a patch I put together. I thought the Oracle-style 'CREATE OR > REPLACE FUNCTION' syntax might be useful to those doing lots of function > creation/development. It is against the 7.1.3 source. > > I ran it through regress - all tests were fine. I also modified the > regress to use the potential OR REPLACE syntax. This also worked. > > Gavin Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) -- 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, Pennsylvania 19026