Thread

1. Outstanding patches

   Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-08T19:57:24Z

   OK, now that we have started 7.2 development, I am going to go through
   the outstanding patches and start to apply them or reject them.  They
   are at:
   
   	http://candle.pha.pa.us/cgi-bin/pgpatches
   
   I could use help in identifying which patches are a problem.  Most of
   the ones there now have been reviewed by me or have received the
   recommendation of another developer.
   
   I particularly need JDBC help because I have many JDBC patches.
   
   If you would send email stating which patches should _not_ be applied, I
   would appreciate it.
   
   Of course, patches can always be backed out.
   
   -- 
     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
   
   

2. Re: Outstanding patches

   Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-08T21:20:05Z

   Bruce Momjian <pgman@candle.pha.pa.us> writes:
   > OK, now that we have started 7.2 development, I am going to go through
   > the outstanding patches and start to apply them or reject them.  They
   > are at:
   > 	http://candle.pha.pa.us/cgi-bin/pgpatches
   > I could use help in identifying which patches are a problem.  Most of
   > the ones there now have been reviewed by me or have received the
   > recommendation of another developer.
   
   Okay, I looked through these ...
   
   I do not like Ian Taylor's plpgsql cursor patch; trying to do cursors
   inside plpgsql with no SPI-level support is too much of a kluge.  We
   should first add cursor support to SPI, then fix plpgsql.  Much of the
   parsing work he's done could be salvaged, but the implementation can't
   be.  (And I don't want to apply it now and back it out later --- it adds
   too many warts.)
   
   Fernando Nasser's ANALYZE patch is superseded by already-applied work,
   though if he wants to do the promised test additions I would be happy.
   
   The PAM support patch concerns me --- it looks like yet another chunk
   of code that will tie up the postmaster in a single-threaded
   conversation with a remote daemon that may or may not respond promptly.
   I recommend holding off on this until we think about whether we
   shouldn't restructure the postmaster to do all authentication work in
   per-client subprocesses.
   
   We need to discuss whether we like the %TYPE feature proposed by Ian
   Taylor.  It seems awfully nonstandard to me, and I'm not sure that the
   value is great enough to be worth inventing a nonstandard feature.
   ISTM that people don't normally tie functions to tables so tightly that
   it's better to define a function in terms of "the type of column foo
   of table bar" than just in terms of the type itself.  Ian claims that
   this is helpful, but is it really likely that you can change that column
   type without making *any* other mods to the function?  Moreover, in
   exchange for this possible benefit you are opening yourself to breaking
   the function if you choose to rename either the column or the table.
   The potential net gain seems really small.  (If we do like the
   functionality, then the patch itself seems OK with the exception of the
   gram.y definition of func_type; the table name should be TokenId not
   just IDENT.)
   
   I did not look at any of the JDBC or libpq++ patches.  The other stuff
   seemed OK on a first glance.
   
   			regards, tom lane
   
   

3. Re: Outstanding patches

   Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-08T21:35:39Z

   > Okay, I looked through these ...
   
   Thanks.
   
   > I do not like Ian Taylor's plpgsql cursor patch; trying to do cursors
   > inside plpgsql with no SPI-level support is too much of a kluge.  We
   > should first add cursor support to SPI, then fix plpgsql.  Much of the
   > parsing work he's done could be salvaged, but the implementation can't
   > be.  (And I don't want to apply it now and back it out later --- it adds
   > too many warts.)
   
   I know Jan is talking about SPI support fpr plpgsql.  I will keep the
   patch but not apply it.
   
   > Fernando Nasser's ANALYZE patch is superseded by already-applied work,
   > though if he wants to do the promised test additions I would be happy.
   
   I have already emailed him to say you did it already.  Removed.
   
   > The PAM support patch concerns me --- it looks like yet another chunk
   > of code that will tie up the postmaster in a single-threaded
   > conversation with a remote daemon that may or may not respond promptly.
   > I recommend holding off on this until we think about whether we
   > shouldn't restructure the postmaster to do all authentication work in
   > per-client subprocesses.
   
   I have not idea what PAM is.  If it is a valuable feature, we can
   install it.  But if it is yet another authentication scheme, it could
   add more confusion to our already complicated setup.  Seems you are
   saying it is the latter, which is fine with me.
   
   
   > We need to discuss whether we like the %TYPE feature proposed by Ian
   > Taylor.  It seems awfully nonstandard to me, and I'm not sure that the
   > value is great enough to be worth inventing a nonstandard feature.
   > ISTM that people don't normally tie functions to tables so tightly that
   > it's better to define a function in terms of "the type of column foo
   > of table bar" than just in terms of the type itself.  Ian claims that
   > this is helpful, but is it really likely that you can change that column
   > type without making *any* other mods to the function?  Moreover, in
   > exchange for this possible benefit you are opening yourself to breaking
   > the function if you choose to rename either the column or the table.
   > The potential net gain seems really small.  (If we do like the
   > functionality, then the patch itself seems OK with the exception of the
   > gram.y definition of func_type; the table name should be TokenId not
   > just IDENT.)
   
   I thought it was valuable.  I know in Informix 4gl you can set variables
   to track column types, and it helps, especially when you make a column
   longer or something. It also better documents the variable.  I remember
   someone else stating this was a nice feature, so I am inclinded to apply
   it, with your suggested changes.
   
   > I did not look at any of the JDBC or libpq++ patches.  The other stuff
   > seemed OK on a first glance.
   
   Ditto.  JDBC will need comment from JDBC people.  The libpq++ stuff
   looks pretty good to me.
   
   -- 
     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
   
   

4. Re: Outstanding patches

   Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-08T21:49:16Z

   Bruce Momjian <pgman@candle.pha.pa.us> writes:
   >> We need to discuss whether we like the %TYPE feature proposed by Ian
   >> Taylor.  It seems awfully nonstandard to me, and I'm not sure that the
   >> value is great enough to be worth inventing a nonstandard feature.
   >> ISTM that people don't normally tie functions to tables so tightly that
   >> it's better to define a function in terms of "the type of column foo
   >> of table bar" than just in terms of the type itself.  Ian claims that
   >> this is helpful, but is it really likely that you can change that column
   >> type without making *any* other mods to the function?  Moreover, in
   >> exchange for this possible benefit you are opening yourself to breaking
   >> the function if you choose to rename either the column or the table.
   >> The potential net gain seems really small.  (If we do like the
   >> functionality, then the patch itself seems OK with the exception of the
   >> gram.y definition of func_type; the table name should be TokenId not
   >> just IDENT.)
   
   > I thought it was valuable.  I know in Informix 4gl you can set variables
   > to track column types, and it helps, especially when you make a column
   > longer or something. It also better documents the variable.
   
   But it's not really tracking the variable; with Ian's proposed
   implementation, after
   
   		create table foo(bar int4);
   
   		create function fooey(foo.bar%type) ...;
   
   		drop table foo;
   
   		create table foo(bar int8);
   
   you would still have fooey declared as taking int4 not int8, because
   the type meant by %type is resolved and frozen immediately upon being
   seen.
   
   Moreover, because of our function-name-overloading feature, fooey(int4)
   and fooey(int8) are two different functions.  IMHO it would be a bad
   thing if we *did* try to change the signature.  We'd break existing
   callers of the function, not to mention possibly run into a naming
   conflict with a pre-existing fooey(int8).
   
   I presume that Ian is not thinking about such a scenario, but only about
   using %type in a schema file that he will reload into a freshly created
   database each time he edits it.  That avoids the issue of whether %type
   declarations can or should track changes on the fly, but I think he's
   still going to run into problems with function naming: do
   fooey(foo.bar%type) and fooey(foo.baz%type) conflict, or not?  Maybe
   today the schema works and tomorrow you get an error.
   
   Basically I think that this feature does not coexist well with function
   overloading, and that it's likely to create as much or more grief as it
   avoids.
   
   			regards, tom lane
   
   

5. Re: Outstanding patches

   Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-08T21:55:54Z

   > I presume that Ian is not thinking about such a scenario, but only about
   > using %type in a schema file that he will reload into a freshly created
   > database each time he edits it.  That avoids the issue of whether %type
   > declarations can or should track changes on the fly, but I think he's
   > still going to run into problems with function naming: do
   > fooey(foo.bar%type) and fooey(foo.baz%type) conflict, or not?  Maybe
   > today the schema works and tomorrow you get an error.
   > 
   > Basically I think that this feature does not coexist well with function
   > overloading, and that it's likely to create as much or more grief as it
   > avoids.
   
   But don't we already have problems with changing functions that use
   tables or does this open a new type of problem?  Seems if you define a
   function to be based on a column, and the column changes, the person
   should expect errors.
   
   If we define things as %TYPE in plpgsql, do we handle cases when the
   column type changes?
   
   -- 
     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
   
   

6. Re: Outstanding patches

   Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-08T22:08:07Z

   Bruce Momjian <pgman@candle.pha.pa.us> writes:
   > But don't we already have problems with changing functions that use
   > tables or does this open a new type of problem?
   
   But this feature isn't about functions that use tables internally;
   it's about tying the fundamental signature of the function to a table.
   I doubt that that's a good idea.  It definitely does introduce potential
   for problems that weren't there before, per the illustrations I already
   gave.
   
   You commented earlier that it's easy to "change the width of a column"
   with this approach, but that's irrelevant, because atttypmod is not part
   of a function's signature anyhow.
   
   > If we define things as %TYPE in plpgsql, do we handle cases when the
   > column type changes?
   
   Sort of, because we just need to drop the cached precompiled version of
   the function --- you can do that by starting a fresh backend if nothing
   else, and we have speculated about making it happen automatically.
   Changing a function's signature automatically is a MUCH bigger and
   nastier can of worms, because it affects things outside the function.
   
   			regards, tom lane
   
   

7. Re: Re: Outstanding patches

   Richard Poole <richard.poole@vi.net> — 2001-05-08T22:15:53Z

   On Tue, May 08, 2001 at 05:49:16PM -0400, Tom Lane wrote:
   > I presume that Ian is not thinking about such a scenario, but only about
   > using %type in a schema file that he will reload into a freshly created
   > database each time he edits it.  That avoids the issue of whether %type
   > declarations can or should track changes on the fly, but I think he's
   > still going to run into problems with function naming: do
   > fooey(foo.bar%type) and fooey(foo.baz%type) conflict, or not?  Maybe
   > today the schema works and tomorrow you get an error.
   
   How about a feature in psql which would read something like '%type' and
   convert it to the appropriate thing before it passed it to the backend?
   Then you could use it without thinking about it in a script which you
   would \i into psql. That would do what's wanted here without having
   any backend nasties. I'm not offering to implement it myself - at least
   not right now - but does it seem like a sensible idea?
   
   Richard
   
   

8. Re: Re: Outstanding patches

   Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-08T22:24:18Z

   Richard Poole <richard.poole@vi.net> writes:
   > How about a feature in psql which would read something like '%type' and
   > convert it to the appropriate thing before it passed it to the backend?
   
   That's just about what Ian's patch does, only it does it during backend
   parsing instead of in the client.  It seems to me that most of the
   arguments against it apply either way.
   
   If we are going to have it, the backend is certainly the right place to
   do it, rather than adding a huge amount of new smarts to psql.
   
   			regards, tom lane
   
   

9. Re: Re: Outstanding patches

   Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-08T22:43:45Z

   > But this feature isn't about functions that use tables internally;
   > it's about tying the fundamental signature of the function to a table.
   > I doubt that that's a good idea.  It definitely does introduce potential
   > for problems that weren't there before, per the illustrations I already
   > gave.
   > 
   > You commented earlier that it's easy to "change the width of a column"
   > with this approach, but that's irrelevant, because atttypmod is not part
   > of a function's signature anyhow.
   
   Yea, that is more an Informix issue.
   
   > > If we define things as %TYPE in plpgsql, do we handle cases when the
   > > column type changes?
   > 
   > Sort of, because we just need to drop the cached precompiled version of
   > the function --- you can do that by starting a fresh backend if nothing
   > else, and we have speculated about making it happen automatically.
   > Changing a function's signature automatically is a MUCH bigger and
   > nastier can of worms, because it affects things outside the function.
   
   OK, one idea is to throw a elog(NOTICE) when they use this feature,
   stating that it will not track column changes.  Another option is to
   just forget about the feature entirely.  Do we have people who like this
   feature?  Speak up now.  If not, we will drop it.
   
   -- 
     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
   
   

10. Re: Re: Outstanding patches

    Ian Lance Taylor <ian@airs.com> — 2001-05-09T01:02:17Z

    Tom Lane <tgl@sss.pgh.pa.us> writes:
    
    > I do not like Ian Taylor's plpgsql cursor patch; trying to do cursors
    > inside plpgsql with no SPI-level support is too much of a kluge.  We
    > should first add cursor support to SPI, then fix plpgsql.  Much of the
    > parsing work he's done could be salvaged, but the implementation can't
    > be.  (And I don't want to apply it now and back it out later --- it adds
    > too many warts.)
    
    I think most of the cursor patch will stand even after SPI-level
    support for cursors is added.  But it's up to you, of course.  7.2 is
    a long way away in any case.  I would be happy to rework the patch
    when SPI supports cursors.
    
    > We need to discuss whether we like the %TYPE feature proposed by Ian
    > Taylor.  It seems awfully nonstandard to me, and I'm not sure that the
    > value is great enough to be worth inventing a nonstandard feature.
    
    Oracle PL/SQL supports this, and PL/SQL code that I've seen uses it
    extensively.  PL/pgSQL supports %TYPE in all places a type may be
    used, except parameter and return types.
    
    > ISTM that people don't normally tie functions to tables so tightly that
    > it's better to define a function in terms of "the type of column foo
    > of table bar" than just in terms of the type itself.  Ian claims that
    > this is helpful, but is it really likely that you can change that column
    > type without making *any* other mods to the function?
    
    Sure.  I've seen code in which all access to the database is done via
    stored procedures.  It's natural to write that sort of code using
    %TYPE.  Otherwise any change you make to the schema, you have to make
    two or three times.
    
    Admittedly, this may apply mostly to what Postgres calls type
    modifiers.  But it's still a natural way to write the procedure.  Why
    duplicate information?
    
    > Moreover, in
    > exchange for this possible benefit you are opening yourself to breaking
    > the function if you choose to rename either the column or the table.
    
    If you do that you've most likely broken the function anyhow, since
    you probably wouldn't use %TYPE if you weren't referring to the
    column.  Anyhow, if you don't use %TYPE you can break the function in
    the other way, by changing the type of the column.  So I think it's
    six of one, half-dozen of the other.
    
    > (If we do like the
    > functionality, then the patch itself seems OK with the exception of the
    > gram.y definition of func_type; the table name should be TokenId not
    > just IDENT.)
    
    I think I tried that, and I think it led to lots of reduce/reduce
    errors.  But maybe that was only in 7.0.3.
    
    The problem that the function type does not change when the schema
    changes is problematical.  I would have been happier if I could have
    left the %TYPE as a string to be interpreted at execution time.  But
    of course that does not work with the current system for function
    overloading.
    
    Ian
    
    ---------------------------(end of broadcast)---------------------------
    TIP 483: In Lowes Crossroads, Delaware, it is a violation of local law
    for any pilot or passenger to carry an ice cream cone in their pocket
    while either flying or waiting to board a plane.
    
    

11. Re: Outstanding patches

    Alessio Bragadini <alessio@albourne.com> — 2001-05-09T07:18:12Z

    Tom Lane wrote:
    
    > But it's not really tracking the variable; with Ian's proposed
    > implementation, after
    > 
    >                 create table foo(bar int4);
    > 
    >                 create function fooey(foo.bar%type) ...;
    > 
    >                 drop table foo;
    > 
    >                 create table foo(bar int8);
    > 
    > you would still have fooey declared as taking int4 not int8, because
    > the type meant by %type is resolved and frozen immediately upon being
    > seen.
    
    Ok, this is a more general point: in Oracle (which, as Ian points out,
    uses this feature extensively) if you recreate table foo, function fooey
    is tagged as 'dirty' and recompiled on the spot next time is used. This
    is also true for VIEWs and other objects, so you don't have the problem
    we have when a view breaks because you've updated the underlining table.
    
    -- 
    Alessio F. Bragadini		alessio@albourne.com
    APL Financial Services		http://village.albourne.com
    Nicosia, Cyprus		 	phone: +357-2-755750
    
    "It is more complicated than you think"
    		-- The Eighth Networking Truth from RFC 1925
    
    

12. Re: Outstanding patches

    Philip Warner <pjw@rhyme.com.au> — 2001-05-09T10:17:13Z

    Is anybody planning to fix the problem with ALTER TABLE ADD CONSTRAINT...
    in which the constraints are not applied to child tables?
    
    
    ----------------------------------------------------------------
    Philip Warner                    |     __---_____
    Albatross Consulting Pty. Ltd.   |----/       -  \
    (A.B.N. 75 008 659 498)          |          /(@)   ______---_
    Tel: (+61) 0500 83 82 81         |                 _________  \
    Fax: (+61) 0500 83 82 82         |                 ___________ |
    Http://www.rhyme.com.au          |                /           \|
                                     |    --________--
    PGP key available upon request,  |  /
    and from pgp5.ai.mit.edu:11371   |/
    
    

13. Re: Outstanding patches

    Richard Bullington-McGuire <rbulling@microstate.com> — 2001-05-09T12:03:18Z

    On Tue, 8 May 2001, Bruce Momjian wrote:
    
    > > The PAM support patch concerns me --- it looks like yet another chunk
    > > of code that will tie up the postmaster in a single-threaded
    > > conversation with a remote daemon that may or may not respond promptly.
    > > I recommend holding off on this until we think about whether we
    > > shouldn't restructure the postmaster to do all authentication work in
    > > per-client subprocesses.
    >
    > I have not idea what PAM is.  If it is a valuable feature, we can
    > install it.  But if it is yet another authentication scheme, it could
    > add more confusion to our already complicated setup.  Seems you are
    > saying it is the latter, which is fine with me.
    
    PAM is a universal interface to many authentication schemes. If PostgreSQL
    supports PAM properly, it can instantly support many different types of
    authentication, such as UNIX, Kerberos, RADIUS, LDAP, or even
    Windows NT domain authentication. Solaris and most modern Linux
    distributions (certainly Red Hat) support PAM:
    
    http://www.sun.com/solaris/pam/
    http://www.kernel.org/pub/linux/libs/pam/
    
    PAM modules are very flexible -- they are even stackable. I've used PAM to
    allow the UW IMAP server running on Red Hat Linux to get its passwords
    either from UNIX authentication or from a Windows NT server, for example.
    
    Given that this has the potential to reduce the number of places that
    system administrators have to maintain passwords, I'd call it a win
    overall, except for that pesky single-threaded issue. You should keep in
    mind, though, that some PAM calls won't involve calls to daemons that
    might not be responsive. Let's say PAM is configured to check
    UNIX authentication (/etc/passwd and /etc/shadow) for passwords -- there
    is no daemon involved, just calls to C libraries that will return
    promptly. If the PAM config file had something like LDAP authentication
    indicated, you would have a potential issue if the LDAP server did not
    respond.
    
    As long as this limitation was documented, though, this would be a very
    valuable addition. A release note saying that the feature was
    experimental, and outlining the limitations in the face of choosing an
    authentication scheme that may fail to answer might be appropriate.
    
     --
     Richard Bullington-McGuire  <rbulling@microstate.com>
     Chief Technology Officer, The Microstate Corporation
     Phone: 703-796-6446  URL: http://www.microstate.com/
     PGP key IDs:    RSA: 0x93862305   DH/DSS: 0xDAC3028E
    
    
    
    

14. Re: Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-09T13:36:07Z

    > 
    > Is anybody planning to fix the problem with ALTER TABLE ADD CONSTRAINT...
    > in which the constraints are not applied to child tables?
    
    I thought we had not figured out how to inherit those, or at least
    certain constraints like UNIQUE.  We do have on TODO:
    
    	* Allow inherited tables to inherit index, UNIQUE constraint, and
    	primary key [inheritance]
    
    -- 
      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
    
    

15. Re: Outstanding patches

    Philip Warner <pjw@rhyme.com.au> — 2001-05-09T14:29:41Z

    At 09:36 9/05/01 -0400, Bruce Momjian wrote:
    >> 
    >> Is anybody planning to fix the problem with ALTER TABLE ADD CONSTRAINT...
    >> in which the constraints are not applied to child tables?
    >
    >I thought we had not figured out how to inherit those, or at least
    >certain constraints like UNIQUE.  We do have on TODO:
    >
    >	* Allow inherited tables to inherit index, UNIQUE constraint, and
    >	primary key [inheritance]
    >
    
    aaa=# create table t1(f1 integer check(f1<>0),primary key (f1));
    aaa=# create table t1c() inherits (t1);
    aaa=# \d t1c
              Table "t1c"
     Attribute |  Type   | Modifier
    -----------+---------+----------
     f1        | integer | not null
    Constraint: (f1 <> 0)
    
    So PK is not inherited, but CHECK (and implied NOT NULL) seem to be.
    
    Whereas,
    
    aaa=# create table t1(f1 integer);
    aaa=# create table t1c() inherits (t1);
    aaa=# alter table t1 add constraint aaa check(f1<>0);
    aaa=# \d t1c
              Table "t1c"
     Attribute |  Type   | Modifier
    -----------+---------+----------
     f1        | integer |
    
    ie. The CHECK constraints inherit only at the time of table creation. I
    think this is a bug in  ALTER TABLE for CHECK constraints.
    
    
    ----------------------------------------------------------------
    Philip Warner                    |     __---_____
    Albatross Consulting Pty. Ltd.   |----/       -  \
    (A.B.N. 75 008 659 498)          |          /(@)   ______---_
    Tel: (+61) 0500 83 82 81         |                 _________  \
    Fax: (+61) 0500 83 82 82         |                 ___________ |
    Http://www.rhyme.com.au          |                /           \|
                                     |    --________--
    PGP key available upon request,  |  /
    and from pgp5.ai.mit.edu:11371   |/
    
    

16. Re: Outstanding patches

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-09T15:00:23Z

    Philip Warner <pjw@rhyme.com.au> writes:
    > Is anybody planning to fix the problem with ALTER TABLE ADD CONSTRAINT...
    > in which the constraints are not applied to child tables?
    
    AFAIK no one is looking at it presently (although Stephan Szabo has
    probably thought about it).  If you want to tackle it, step right up,
    but coordinate with Stephan.
    
    I was just in the vicinity of ALTER TABLE, and noted that that routine
    didn't have the same loop-over-children superstructure that most of the
    other ALTER code does.  Should be a relatively simple matter to graft
    that logic onto it, unless there are semantic funnies that come up with
    propagating the new constraint.
    
    			regards, tom lane
    
    

17. Re: Re: Outstanding patches

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-09T15:06:43Z

    Alessio Bragadini <alessio@albourne.com> writes:
    > Tom Lane wrote:
    >> But it's not really tracking the variable; with Ian's proposed
    >> implementation, after
    >> 
    >> create table foo(bar int4);
    >> 
    >> create function fooey(foo.bar%type) ...;
    >> 
    >> drop table foo;
    >> 
    >> create table foo(bar int8);
    >> 
    >> you would still have fooey declared as taking int4 not int8, because
    >> the type meant by %type is resolved and frozen immediately upon being
    >> seen.
    
    > Ok, this is a more general point: in Oracle (which, as Ian points out,
    > uses this feature extensively) if you recreate table foo, function fooey
    > is tagged as 'dirty' and recompiled on the spot next time is used. This
    > is also true for VIEWs and other objects, so you don't have the problem
    > we have when a view breaks because you've updated the underlining table.
    
    Indeed, and we have plans to do something similar sometime soon.  My
    real objection to this proposed feature is that there is no way to
    handle the update as a local matter within the function, because
    changing the function's input datatypes actually means it's a different
    function.  This creates all sorts of problems at both the definitional
    and implementation levels...
    
    			regards, tom lane
    
    

18. Re: Outstanding patches

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-09T15:21:30Z

    Philip Warner <pjw@rhyme.com.au> writes:
    > ie. The CHECK constraints inherit only at the time of table creation. I
    > think this is a bug in  ALTER TABLE for CHECK constraints.
    
    More like an "unimplemented feature" ;-).
    
    After thinking for a moment, I believe the only real gotcha that could
    arise here is to make sure that the constraint is adjusted for the
    possibly-different column numbers in each child table.  There is code
    available to make this happen, or it might happen for free if you can
    postpone parse analysis of the raw constraint tree until you are looking
    at each child table.  Just something to keep in mind and test while
    you're doing it ...
    
    			regards, tom lane
    
    

19. Re: Re: Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-09T15:53:45Z

    > > Ok, this is a more general point: in Oracle (which, as Ian points out,
    > > uses this feature extensively) if you recreate table foo, function fooey
    > > is tagged as 'dirty' and recompiled on the spot next time is used. This
    > > is also true for VIEWs and other objects, so you don't have the problem
    > > we have when a view breaks because you've updated the underlining table.
    > 
    > Indeed, and we have plans to do something similar sometime soon.  My
    > real objection to this proposed feature is that there is no way to
    > handle the update as a local matter within the function, because
    > changing the function's input datatypes actually means it's a different
    > function.  This creates all sorts of problems at both the definitional
    > and implementation levels...
    
    Does this relate to allowing functions to be recreated with the same OID
    as the original function?  I think we need that badly for 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, Pennsylvania 19026
    
    

20. Re: Re: Outstanding patches

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-09T16:01:29Z

    Bruce Momjian <pgman@candle.pha.pa.us> writes:
    > Does this relate to allowing functions to be recreated with the same OID
    > as the original function?  I think we need that badly for 7.2.
    
    No, I don't think that's very related; that's a simple matter of
    implementing an ALTER FUNCTION command.  The other thing will require
    figuring out how to do dependency tracking.
    
    			regards, tom lane
    
    

21. Re: Re: Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-09T16:03:16Z

    > Bruce Momjian <pgman@candle.pha.pa.us> writes:
    > > Does this relate to allowing functions to be recreated with the same OID
    > > as the original function?  I think we need that badly for 7.2.
    > 
    > No, I don't think that's very related; that's a simple matter of
    > implementing an ALTER FUNCTION command.  The other thing will require
    > figuring out how to do dependency tracking.
    
    Got it.  Let me ask, if they change the column type, would they use
    ALTER FUNCTION to then update to match the new column type.  As I
    understand it, the problem is that this does not happen automatically,
    right?
    
    -- 
      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
    
    

22. Re: Re: Outstanding patches

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-09T16:07:03Z

    Bruce Momjian <pgman@candle.pha.pa.us> writes:
    >> No, I don't think that's very related; that's a simple matter of
    >> implementing an ALTER FUNCTION command.  The other thing will require
    >> figuring out how to do dependency tracking.
    
    > Got it.  Let me ask, if they change the column type, would they use
    > ALTER FUNCTION to then update to match the new column type.  As I
    > understand it, the problem is that this does not happen automatically,
    > right?
    
    My vision of ALTER FUNCTION is that it would let you change the function
    body, and perhaps also the function language and attributes (isCachable,
    isStrict).  It would NOT allow you to change the function's parameter
    types or return type, because that potentially breaks things that depend
    on the function.  To do that, you should have to create a new function.
    
    			regards, tom lane
    
    

23. Re: Outstanding patches

    Stephan Szabo <sszabo@megazone23.bigpanda.com> — 2001-05-09T16:13:37Z

    On Wed, 9 May 2001, Philip Warner wrote:
    
    > 
    > Is anybody planning to fix the problem with ALTER TABLE ADD CONSTRAINT...
    > in which the constraints are not applied to child tables?
    > 
    
    I'm working on the check constraint case (didn't realize that
    those inherited since unique, primary key and foreign key
    can't right now).  We need to really figure out what we're going to
    do with the other constraints to make them inherit.
    
    
    
    

24. Re: Outstanding patches

    Peter Eisentraut <peter_e@gmx.net> — 2001-05-09T18:59:38Z

    Bruce Momjian writes:
    
    > I could use help in identifying which patches are a problem.  Most of
    > the ones there now have been reviewed by me or have received the
    > recommendation of another developer.
    
    I have a few stylistic issues with the createlang patch, but I can work on
    installing it myself.
    
    -- 
    Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter
    
    
    

25. RE: Outstanding patches

    Christopher Kings-Lynne <chriskl@familyhealth.com.au> — 2001-05-10T01:50:21Z

    I'll have a look-see since it's in the vicinity of the code I'm messing with
    at the moment.  Time is my problem, though...
    
    Chris
    
    > -----Original Message-----
    > From: pgsql-hackers-owner@postgresql.org
    > [mailto:pgsql-hackers-owner@postgresql.org]On Behalf Of Tom Lane
    > Sent: Wednesday, 9 May 2001 11:00 PM
    > To: Philip Warner
    > Cc: PostgreSQL-development
    > Subject: Re: [HACKERS] Outstanding patches
    >
    >
    > Philip Warner <pjw@rhyme.com.au> writes:
    > > Is anybody planning to fix the problem with ALTER TABLE ADD
    > CONSTRAINT...
    > > in which the constraints are not applied to child tables?
    >
    > AFAIK no one is looking at it presently (although Stephan Szabo has
    > probably thought about it).  If you want to tackle it, step right up,
    > but coordinate with Stephan.
    >
    > I was just in the vicinity of ALTER TABLE, and noted that that routine
    > didn't have the same loop-over-children superstructure that most of the
    > other ALTER code does.  Should be a relatively simple matter to graft
    > that logic onto it, unless there are semantic funnies that come up with
    > propagating the new constraint.
    >
    > 			regards, tom lane
    >
    > ---------------------------(end of broadcast)---------------------------
    > TIP 2: you can get off all lists at once with the unregister command
    >     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
    >
    
    
    

26. Re: [HACKERS] Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-10T14:13:58Z

    [ Charset ISO-8859-15 unsupported, converting... ]
    > Bruce Momjian wrote:
    > > 
    > > OK, now that we have started 7.2 development, I am going to go through
    > > the outstanding patches and start to apply them or reject them.  They
    > > are at:
    > > 
    > >         http://candle.pha.pa.us/cgi-bin/pgpatches
    > > 
    > 
    > Has the patch that makes MOVE return number of rows actually moved
    > (analoguous 
    > to UPDATE and DELETE) been properly submitted to patches ?
    
    I know MOVE had fixes in 7.1.  I don't know of any outstanding MOVE
    bugs.
    
    -- 
      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
    
    

27. Re: [HACKERS] Outstanding patches

    Hannu Krosing <hannu@tm.ee> — 2001-05-10T14:14:28Z

    Bruce Momjian wrote:
    > 
    > OK, now that we have started 7.2 development, I am going to go through
    > the outstanding patches and start to apply them or reject them.  They
    > are at:
    > 
    >         http://candle.pha.pa.us/cgi-bin/pgpatches
    > 
    
    Has the patch that makes MOVE return number of rows actually moved
    (analoguous 
    to UPDATE and DELETE) been properly submitted to patches ?
    
    --------------------
    Hannu
    
    

28. Re: Re: [HACKERS] Outstanding patches

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-10T15:07:22Z

    Bruce Momjian <pgman@candle.pha.pa.us> writes:
    >> Has the patch that makes MOVE return number of rows actually moved
    >> (analoguous to UPDATE and DELETE) been properly submitted to patches ?
    
    > I know MOVE had fixes in 7.1.  I don't know of any outstanding MOVE
    > bugs.
    
    It wasn't a bug, it was a feature ;-)
    
    Bruce did not have that patch on his list of things-to-apply, so either
    it was never properly submitted or it slipped through the cracks.
    Anyone want to dig it up and verify it against 7.1?
    
    			regards, tom lane
    
    

29. Re: Re: [HACKERS] Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-10T16:12:46Z

    > Here are the patches. Please look at them, and if you think 
    > it's a good idea, then please let me know where and how should
    > I post them, and approximately when will you finish with the
    > beta testing, so it can be really considered seriously.
    > 
    > I included them also as an attachment, because my silly pine
    > insists to break the lines...
    
    Looks fine to me.  I don't remember ever seeing this before.
    
    > p.s.: I read a page on your homepage, called "unapplied patches".
    > I would like to know if it means "still unapplied patches", or
    > it means "bad, and not accepted ideas".
    
    It means it is probably good, waiting for approval from others, or in
    the standard one-delay before applying patches.
    
    -- 
      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
    
    

30. Re: Re: [HACKERS] Outstanding patches

    Hannu Krosing <hannu@tm.ee> — 2001-05-10T16:14:03Z

    Tom Lane wrote:
    > 
    > Bruce Momjian <pgman@candle.pha.pa.us> writes:
    > >> Has the patch that makes MOVE return number of rows actually moved
    > >> (analoguous to UPDATE and DELETE) been properly submitted to patches ?
    > 
    > > I know MOVE had fixes in 7.1.  I don't know of any outstanding MOVE
    > > bugs.
    > 
    > It wasn't a bug, it was a feature ;-)
    > 
    > Bruce did not have that patch on his list of things-to-apply, so either
    > it was never properly submitted or it slipped through the cracks.
    > Anyone want to dig it up and verify it against 7.1?
    
    I forward it here so you don't have to dig it up:
    -----------------------------------------------------------------------
    Hi.
    
    A few weeks (months?) ago I made a patch to the postgres
    backend to get back the number of realized moves after
    a MOVE command. So if I issue a "MOVE 100 IN cusrorname",
    but there was only 66 rows left, I get back not only "MOVE",
    but "MOVE 66". If the 100 steps could be realized, then
    "MOVE 100" would come back. 
    
    I send this info to you, because I would like to ask you if
    it could be OK to include in future versions. I think you
    are in a beta testing phase now, so it is trivially not the
    time to include it now...
    
    The solution is 2 code lines into command.c, and then the
    message of move cames with the number into for example psql.
    1 other word into the jdbc driver, and this number is
    available at update_count.
    
    I made the patch to the latest (one day old) CVS version.
    
    Here are the patches. Please look at them, and if you think 
    it's a good idea, then please let me know where and how should
    I post them, and approximately when will you finish with the
    beta testing, so it can be really considered seriously.
    
    I included them also as an attachment, because my silly pine
    insists to break the lines...
    
    --- ./src/backend/commands/command.c.orig	Fri Mar 23 05:49:52 2001
    +++ ./src/backend/commands/command.c	Sat Apr  7 10:24:27 2001
    @@ -174,6 +174,12 @@
     		if (!portal->atEnd)
     		{
     			ExecutorRun(queryDesc, estate, EXEC_FOR, (long) count);
    +
    +			/* I use CMD_UPDATE, because no CMD_MOVE or the like
    +			   exists, and I would like to provide the same
    +			   kind of info as CMD_UPDATE */
    +			UpdateCommandInfo(CMD_UPDATE, 0, estate->es_processed);
    +
     			if (estate->es_processed > 0)
     				portal->atStart = false;		/* OK to back up now */
     			if (count <= 0 || (int) estate->es_processed < count)
    @@ -185,6 +191,12 @@
     		if (!portal->atStart)
     		{
     			ExecutorRun(queryDesc, estate, EXEC_BACK, (long) count);
    +
    +			/* I use CMD_UPDATE, because no CMD_MOVE or the like
    +			   exists, and I would like to provide the same
    +			   kind of info as CMD_UPDATE */
    +			UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed);
    +
     			if (estate->es_processed > 0)
     				portal->atEnd = false;	/* OK to go forward now */
     			if (count <= 0 || (int) estate->es_processed < count)
    
    
    
    Here is the patch for the jdbc driver. >! I couldn't test it
    with the current version, because it needs ant, and I didn't
    have time and money today to download it... !< However, it
    is a trivial change, and if Peter T. Mount reads it, I ask
    him to check if he likes it... Thanks for any kind of answer.  
    
    --- ./src/interfaces/jdbc/org/postgresql/Connection.java.orig	Wed Jan 31
    09:26:01 2001
    +++ ./src/interfaces/jdbc/org/postgresql/Connection.java	Sat Apr  7
    16:42:04 2001
    @@ -490,7 +490,7 @@
     			    recv_status =
    pg_stream.ReceiveString(receive_sbuf,8192,getEncoding());
     
     				// Now handle the update count correctly.
    -				if(recv_status.startsWith("INSERT") ||
    recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE")) {
    +				if(recv_status.startsWith("INSERT") ||
    recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE") ||
    recv_status.startsWith("MOVE")) {
     					try {
     						update_count =
    Integer.parseInt(recv_status.substring(1+recv_status.lastIndexOf(' ')));
     					} catch(NumberFormatException nfe) {
    
    
    -------------------
    (This last looks a bit complex, but the change is really a new 
    "|| recv_status.startsWith("MOVE")" only...)
    
    
    Thank you for having read this, 
    
    Baldvin
    
    p.s.: I read a page on your homepage, called "unapplied patches".
    I would like to know if it means "still unapplied patches", or
    it means "bad, and not accepted ideas".
    
    

31. Re: Re: [HACKERS] Outstanding patches

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-05-10T20:52:49Z

    > +			/* I use CMD_UPDATE, because no CMD_MOVE or the like
    > +			   exists, and I would like to provide the same
    > +			   kind of info as CMD_UPDATE */
    > +			UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed);
    
    I do not think it is a good idea to return a negative count for a
    backwards move; that is too likely to break client code that parses
    command result strings and isn't expecting minus signs.  The client
    should know whether he issued MOVE FORWARD or MOVE BACKWARDS anyway,
    so just returning es_processed ought to be sufficient.
    
    Otherwise I think the patch is probably OK.
    
    			regards, tom lane
    
    

32. Re: Re: [HACKERS] Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-12T21:38:39Z

    [ Charset ISO-8859-15 unsupported, converting... ]
    > Bruce Momjian wrote:
    > > 
    > > OK, now that we have started 7.2 development, I am going to go through
    > > the outstanding patches and start to apply them or reject them.  They
    > > are at:
    > > 
    > >         http://candle.pha.pa.us/cgi-bin/pgpatches
    > > 
    > 
    > Has the patch that makes MOVE return number of rows actually moved
    > (analoguous 
    > to UPDATE and DELETE) been properly submitted to patches ?
    
    Yes, it is on the page to be applied, but the page also has Tom Lane's
    objection to returning a negative value.  Can you fix that and resubmit?
    
    -- 
      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
    
    

33. Re: Re: Outstanding patches

    Jan Wieck <janwieck@yahoo.com> — 2001-05-15T15:41:51Z

    Ian Lance Taylor wrote:
    >
    > Oracle PL/SQL supports this, and PL/SQL code that I've seen uses it
    > extensively.  PL/pgSQL supports %TYPE in all places a type may be
    > used, except parameter and return types.
    
        It's  not  PL/pgSQL's  fault  here.  The  pg_proc entries are
        created by the CREATE FUNCTION utility  command  that's  used
        for  all languages.  So what we're talking about affects SQL,
        C, PL/Tcl, PL/Perl, PL/Python and whatnot too. PL/pgSQL might
        live  with that very well, because it has some automatic type
        conversion (using the actual values typoutput and the  needed
        types  typinput functions) to convert values on the fly.  But
        a C function receiving a different type all of a sudden is  a
        good candidate to coredump the backend.
    
    
    Jan
    
    --
    
    #======================================================================#
    # It's easier to get forgiveness for being wrong than for being right. #
    # Let's break this rule - forgive me.                                  #
    #================================================== JanWieck@Yahoo.com #
    
    
    
    _________________________________________________________
    Do You Yahoo!?
    Get your free @yahoo.com address at http://mail.yahoo.com
    
    
    

34. Re: Re: [HACKERS] Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-05-28T14:14:55Z

    Can this patch be resubmitted with a postive-only return value?
    
    
    > > +			/* I use CMD_UPDATE, because no CMD_MOVE or the like
    > > +			   exists, and I would like to provide the same
    > > +			   kind of info as CMD_UPDATE */
    > > +			UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed);
    > 
    > I do not think it is a good idea to return a negative count for a
    > backwards move; that is too likely to break client code that parses
    > command result strings and isn't expecting minus signs.  The client
    > should know whether he issued MOVE FORWARD or MOVE BACKWARDS anyway,
    > so just returning es_processed ought to be sufficient.
    > 
    > Otherwise I think the patch is probably OK.
    > 
    > 			regards, tom lane
    > 
    > ---------------------------(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
    
    

35. Re: Re: [HACKERS] Outstanding patches

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-06-07T00:08:03Z

    > > +			/* I use CMD_UPDATE, because no CMD_MOVE or the like
    > > +			   exists, and I would like to provide the same
    > > +			   kind of info as CMD_UPDATE */
    > > +			UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed);
    > 
    > I do not think it is a good idea to return a negative count for a
    > backwards move; that is too likely to break client code that parses
    > command result strings and isn't expecting minus signs.  The client
    > should know whether he issued MOVE FORWARD or MOVE BACKWARDS anyway,
    > so just returning es_processed ought to be sufficient.
    > 
    > Otherwise I think the patch is probably OK.
    
    I have applied this patch with does MOVE output for both the backend and
    jdbc.  I tested the JDBC patch by compiling, and changed the backend to
    only output postitive values.
    
    Thanks.
    
    -- 
      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