pg_dump, problem with user defined types?

Hi All,

I can't confirm this against the current latest CVS as my
local tree has undergone some changes, but we seem to have
a problem with "pg_dump" and user defined types.

If I create a type like so:-

CREATE FUNCTION sotime_in (opaque ) RETURNS opaque AS
'/usr/local/pgsql/shlibs/obj/sotimes.so' LANGUAGE 'C';

CREATE FUNCTION sotime_out (opaque ) RETURNS opaque AS
'/usr/local/pgsql/shlibs/obj/sotimes.so' LANGUAGE 'C';

CREATE TYPE sotime ( internallength = 2, externallength = 5,
input = sotime_in, output = sotime_out, send = sotime_out,
receive = sotime_in, default = '-',passedbyvalue);

pg_dump gives me back :-

CREATE TYPE sotime ( internallength = 2, externallength = 5,
input = sotime_in_18272, output = sotime_out_18304, send = sotime_out_18304,
receive = sotime_in_18272, default = '-',passedbyvalue);

CREATE TYPE _sotime ( internallength = -1, externallength = -1,
input = array_in_750, output = array_out_751, send = array_out_751,
receive = array_in_750, default = '-');
.
<table creates>
.
CREATE FUNCTION sotime_in (opaque ) RETURNS opaque AS
'/usr/local/pgsql/shlibs/obj/sotimes.so' LANGUAGE 'C';
CREATE FUNCTION sotime_out (opaque ) RETURNS opaque AS
'/usr/local/pgsql/shlibs/obj/sotimes.so' LANGUAGE 'C';

Looks like the FUNCTION creates are OK but the TYPE creation
is a little confused.

I remember some discussion about name mangling for the functions
but it looks like pg_dump can't cope with this.

Another thing I can't understand is pg_dump dumping the _sotime
type, which seems to be created automatically when I create
the sotime type.

Needless to say, I can't dump/restore a database with user defined
types at all.

Yes, we have a problem with user-defined types because of the changes I
made to make the function name unique by adding the pg_proc oid to the
name.

I can make the fix, but no one seems to have an opinion on how yet. I
can e-mail the pertinent postings if you want to discuss solutions.

-- 
  Bruce Momjian                        |  maillist@candle.pha.pa.us
  830 Blythe Avenue                    |  http://www.op.net/~candle
  Drexel Hill, Pennsylvania 19026      |  (610) 353-9879(w)
  +  If your life is a hard drive,     |  (610) 853-3000(h)
  +  Christ can be your backup.        |  

Bruce,

If you have the relevant discussions in a neat bundle I'd like
to have a read through them to see what the issues are.

The other curiosity is the creation of an _<typname> type, which
I believe is an array type, at the time of type creation.

I can't remember seeing this in previous versions.

Thanks,
Keith.

Bruce,

If you have the relevant discussions in a neat bundle I'd like
to have a read through them to see what the issues are.

The other curiosity is the creation of an _<typname> type, which
I believe is an array type, at the time of type creation.

I can't remember seeing this in previous versions.

Thanks,
Keith.

OK, here are the relivant postings. Please make a suggestion for a fix.

-- 
  Bruce Momjian                        |  maillist@candle.pha.pa.us
  830 Blythe Avenue                    |  http://www.op.net/~candle
  Drexel Hill, Pennsylvania 19026      |  (610) 353-9879(w)
  +  If your life is a hard drive,     |  (610) 853-3000(h)
  +  Christ can be your backup.        |  

Bruce,

What I can't see from the postings, is what incident or problem
prompted the change.

Was there test or query that failed because it chose the wrong
procedure?

Do you have an example which failed with the old code and needed
the changes to make it work?

I see pg_proc has "pronargs" and "proargtypes" columns, is this
not enough to ensure that the correct procedure is called?

Or am I off at a tangent again!!

Keith.

Bruce,

What I can't see from the postings, is what incident or problem
prompted the change.

Was there test or query that failed because it chose the wrong
procedure?

Do you have an example which failed with the old code and needed
the changes to make it work?

I see pg_proc has "pronargs" and "proargtypes" columns, is this
not enough to ensure that the correct procedure is called?

Or am I off at a tangent again!!

I found that the regprocin routine was doing a sequential scan of
pg_proc, looking for a function matching the supplied name, and using
the first matching entry of pg_proc.proname.

Two problems: You can't use the system cache to look up the pg_proc
value, because the cache only does unique lookups. Second, we have many
functions that have multiple entries in the pg_proc table with the same
name, but different arguments, so it was not really accurate.

We certainly need to change what I have done, but I am not sure how to
change it.

We can put it back to the old code, or move everything to use only an
oid, with no name, or we can somehow allow the user to supply the
pg_proc.proname, and the argument types, and do a match that way. We
could allow just the proname if there is only one entry with that
proname. (Sequtial scan required, but not too bad. We could even use
the existing index. It is not done that much.) If it is not unique, we
would require the oid.

I hope there is some good solution.

The code currently supports input of oid, or proname_oid, and outputs
proname_oid.

-- 
  Bruce Momjian                        |  maillist@candle.pha.pa.us
  830 Blythe Avenue                    |  http://www.op.net/~candle
  Drexel Hill, Pennsylvania 19026      |  (610) 353-9879(w)
  +  If your life is a hard drive,     |  (610) 853-3000(h)
  +  Christ can be your backup.        |  

pg_dump gives me back :-
CREATE TYPE sotime ( internallength = 2, externallength = 5,
input = sotime_in_18272, output = sotime_out_18304, send = sotime_out_18304,
receive = sotime_in_18272, default = '-',passedbyvalue);
CREATE TYPE _sotime ( internallength = -1, externallength = -1,
input = array_in_750, output = array_out_751, send = array_out_751,
receive = array_in_750, default = '-');

Yes, we have a problem with user-defined types because of the changes
I made to make the function name unique by adding the pg_proc oid to
the name.
I can make the fix, but no one seems to have an opinion on how yet.

It would seem that for Keith's cases things worked OK originally? There
was trouble with other overloaded functions?

At worst, we should revert to the non-mangled names. On a related note,
has anyone had time to try testing pg_upgrade? That would be a nice
feature for v6.4 if it worked on a wide range of databases (haven't
tried it myself yet).

- Tom

What I can't see from the postings, is what incident or problem
prompted the change.
Was there test or query that failed because it chose the wrong
procedure?
Do you have an example which failed with the old code and needed
the changes to make it work?

I found that the regprocin routine was doing a sequential scan of
pg_proc, looking for a function matching the supplied name, and using
the first matching entry of pg_proc.proname.
Two problems: You can't use the system cache to look up the pg_proc
value, because the cache only does unique lookups. Second, we have
many functions that have multiple entries in the pg_proc table with
the same name, but different arguments, so it was not really accurate.
We certainly need to change what I have done, but I am not sure how to
change it.

I'm sorry Bruce for not keeping up, but could you please send me the old
postings which summarize the symptoms of the problem? And any private
response to Keith which answered his initial questions above?

I understand that the regprocin support routine was apparently not using
enough keys in the pg_proc lookup to find the unique entry match, but
where is regprocin used? I assume that it is support code, but is it
used internally only, or are you using it for some of the psql/pg_dump
construction?

From the code used in the parser to support functions and operators, I

would have guessed that you can define multiple key fields for cache
lookups; do those techniques fail in this case or are they not being
used?

Anyway, I'm concerned about the name mangling and the proximity to the
v6.4 release, and would like to help work out the issues if I can...

- Tom

Looks like I am going to need some help here.
The old code dumped out regproc fields as the pg_proc.proname.
There is a problem with this. First, you can have multiple proname
entries with the same proname. The differ in their argument
number/types. The old code, when reading in a regproc name, would do
a sequential scan of the pg_proc table, and find the first entry that
matches the given proname.
If that is not the one you wanted, too bad. No way to change it.

Hi Bruce. I'm sorry again for being so slow, but I'm still not
understanding the initial conditions which prompted these changes. Are
you fixing something proactively, or was there a specific example of
misbehavior? The example I see in your mail with Tatsuo which now causes
trouble is for type input and output routine names, which _are_ likely
to be unique.

Would it be possible for you to bracket the code in the cvs tree so that
we can enable/disable the old behavior? That way we can see what has
changed and how it used to behave. I suppose that would involve
bracketing code in regprocin/out and in pg_dump??

TIA

- Tom

Looks like I am going to need some help here.
The old code dumped out regproc fields as the pg_proc.proname.
There is a problem with this. First, you can have multiple proname
entries with the same proname. The differ in their argument
number/types. The old code, when reading in a regproc name, would do
a sequential scan of the pg_proc table, and find the first entry that
matches the given proname.
If that is not the one you wanted, too bad. No way to change it.

Hi Bruce. I'm sorry again for being so slow, but I'm still not
understanding the initial conditions which prompted these changes. Are
you fixing something proactively, or was there a specific example of
misbehavior? The example I see in your mail with Tatsuo which now causes
trouble is for type input and output routine names, which _are_ likely
to be unique.

Would it be possible for you to bracket the code in the cvs tree so that
we can enable/disable the old behavior? That way we can see what has
changed and how it used to behave. I suppose that would involve
bracketing code in regprocin/out and in pg_dump??

Proactive fix. See my other posting today that has an idea to roll back
the old behavour, while making sure the regproc name is unique.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@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