
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

Tue, 8 May 2001 15:57:24 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Outstanding patches
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

Tue, 08 May 2001 17:20:05 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Outstanding patches 
> 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

Tue, 8 May 2001 17:35:39 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Outstanding patches
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

Tue, 08 May 2001 17:49:16 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Outstanding patches 
> 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

Tue, 8 May 2001 17:55:54 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Outstanding patches
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

Tue, 08 May 2001 18:08:07 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Outstanding patches 
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

Tue, 8 May 2001 23:15:53 +0100
Richard Poole <richard.poole@vi.net>
Re: Re: Outstanding patches
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

Tue, 08 May 2001 18:24:18 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Re: Outstanding patches 
> 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

Tue, 8 May 2001 18:43:45 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: Outstanding patches
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.

08 May 2001 18:02:17 -0700
Ian Lance Taylor <ian@airs.com>
Re: Re: Outstanding patches
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

Wed, 09 May 2001 10:18:12 +0300
Alessio Bragadini <alessio@albourne.com>
Re: Outstanding patches

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

Wed, 09 May 2001 20:17:13 +1000
Philip Warner <pjw@rhyme.com.au>
Re: Outstanding patches
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



Wed, 9 May 2001 08:03:18 -0400 (EDT)
Richard Bullington-McGuire <rbulling@microstate.com>
Re: Outstanding patches
> 
> 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

Wed, 9 May 2001 09:36:07 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Outstanding patches
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   |/

Thu, 10 May 2001 00:29:41 +1000
Philip Warner <pjw@rhyme.com.au>
Re: 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

Wed, 09 May 2001 11:00:23 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Outstanding patches 
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

Wed, 09 May 2001 11:06:43 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Re: Outstanding patches 
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

Wed, 09 May 2001 11:21:30 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Outstanding patches 
> > 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

Wed, 9 May 2001 11:53:45 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: Outstanding patches
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

Wed, 09 May 2001 12:01:29 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Re: Outstanding patches 
> 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

Wed, 9 May 2001 12:03:16 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: Outstanding patches
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

Wed, 09 May 2001 12:07:03 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Re: Outstanding patches 

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.



Wed, 9 May 2001 09:13:37 -0700 (PDT)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: Outstanding patches
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


Wed, 9 May 2001 20:59:38 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: Outstanding patches
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)
>


Thu, 10 May 2001 09:50:21 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
RE: Outstanding patches 
[ 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

Thu, 10 May 2001 10:13:58 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] Outstanding patches
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

Thu, 10 May 2001 16:14:28 +0200
Hannu Krosing <hannu@tm.ee>
Re: [HACKERS] Outstanding patches
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

Thu, 10 May 2001 11:07:22 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Re: [HACKERS] Outstanding patches 
> 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

Thu, 10 May 2001 12:12:46 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: [HACKERS] Outstanding patches
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".

Thu, 10 May 2001 18:14:03 +0200
Hannu Krosing <hannu@tm.ee>
Re: Re: [HACKERS] Outstanding patches
> +			/* 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

Thu, 10 May 2001 16:52:49 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Re: [HACKERS] Outstanding patches 
[ 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

Sat, 12 May 2001 17:38:39 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: [HACKERS] Outstanding patches
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


Tue, 15 May 2001 11:41:51 -0400 (EDT)
Jan Wieck <JanWieck@Yahoo.com>
Re: Re: Outstanding patches

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

Mon, 28 May 2001 10:14:55 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: [HACKERS] Outstanding patches
> > +			/* 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

Index: src/backend/commands/command.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/command.c,v
retrieving revision 1.131
diff -c -r1.131 command.c
*** src/backend/commands/command.c	2001/05/30 13:00:03	1.131
--- src/backend/commands/command.c	2001/06/07 00:03:44
***************
*** 176,181 ****
--- 176,187 ----
  		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)
***************
*** 187,192 ****
--- 193,204 ----
  		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, estate->es_processed);
  			if (estate->es_processed > 0)
  				portal->atEnd = false;	/* OK to go forward now */
  			if (count <= 0 || (int) estate->es_processed < count)
Index: src/interfaces/jdbc/org/postgresql/Connection.java
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/Connection.java,v
retrieving revision 1.16
diff -c -r1.16 Connection.java
*** src/interfaces/jdbc/org/postgresql/Connection.java	2001/06/01 20:57:58	1.16
--- src/interfaces/jdbc/org/postgresql/Connection.java	2001/06/07 00:03:56
***************
*** 505,511 ****
  			    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")) {
  					try {
  						update_count = Integer.parseInt(recv_status.substring(1+recv_status.lastIndexOf(' ')));
  					} catch(NumberFormatException nfe) {
--- 505,511 ----
  			    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") || recv_status.startsWith("MOVE")) {
  					try {
  						update_count = Integer.parseInt(recv_status.substring(1+recv_status.lastIndexOf(' ')));
  					} catch(NumberFormatException nfe) {
Wed, 6 Jun 2001 20:08:03 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Re: [HACKERS] Outstanding patches