pg_proc without oid?

Started by Magnus Haganderalmost 19 years ago14 messages
#1Magnus Hagander
magnus@hagander.net

I notice that this patch:
http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/catalog/pg_proc.h.diff?r1=1.443&r2=1.444

inserts a bunch of XML related rows in pg_proc without specifying oid.
This breaks the fmgrtab generator on msvc. Most likely because I didn't
think of that case. But since all other rows in pg_proc.h contain the
oid, I just wanted to check if they're actually supposed to be withuot
oid, or if that was a mistake?

//Magnus

#2Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#1)
Re: pg_proc without oid?

Am Montag, 19. Februar 2007 10:16 schrieb Magnus Hagander:

This breaks the fmgrtab generator on msvc. Most likely because I didn't
think of that case. But since all other rows in pg_proc.h contain the
oid, I just wanted to check if they're actually supposed to be withuot
oid, or if that was a mistake?

It's intentional.

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

#3Magnus Hagander
magnus@hagander.net
In reply to: Peter Eisentraut (#2)
Re: pg_proc without oid?

On Mon, Feb 19, 2007 at 11:25:02AM +0100, Peter Eisentraut wrote:

Am Montag, 19. Februar 2007 10:16 schrieb Magnus Hagander:

This breaks the fmgrtab generator on msvc. Most likely because I didn't
think of that case. But since all other rows in pg_proc.h contain the
oid, I just wanted to check if they're actually supposed to be withuot
oid, or if that was a mistake?

It's intentional.

Could you explain why, and what the expected result is? Since I can't
find any other examples of people doing it :-)

(will fix the vc stuff, of course, but still interested in knowing)

//Magnus

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#2)
Re: pg_proc without oid?

Peter Eisentraut <peter_e@gmx.net> writes:

Am Montag, 19. Februar 2007 10:16 schrieb Magnus Hagander:

This breaks the fmgrtab generator on msvc. Most likely because I didn't
think of that case. But since all other rows in pg_proc.h contain the
oid, I just wanted to check if they're actually supposed to be withuot
oid, or if that was a mistake?

It's intentional.

Kindly change that intention.

regards, tom lane

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#4)
Re: pg_proc without oid?

Am Montag, 19. Februar 2007 16:26 schrieb Tom Lane:

Peter Eisentraut <peter_e@gmx.net> writes:

Am Montag, 19. Februar 2007 10:16 schrieb Magnus Hagander:

This breaks the fmgrtab generator on msvc. Most likely because I didn't
think of that case. But since all other rows in pg_proc.h contain the
oid, I just wanted to check if they're actually supposed to be withuot
oid, or if that was a mistake?

It's intentional.

Kindly change that intention.

What is wrong?

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#5)
Re: pg_proc without oid?

Peter Eisentraut <peter_e@gmx.net> writes:

Am Montag, 19. Februar 2007 16:26 schrieb Tom Lane:

Peter Eisentraut <peter_e@gmx.net> writes:

Am Montag, 19. Februar 2007 10:16 schrieb Magnus Hagander:

This breaks the fmgrtab generator on msvc.

It's intentional.

Kindly change that intention.

What is wrong?

Well, in the first place Gen_fmgrtab.sh is producing garbage:

#define F_CURSOR_TO_XML DATAINSERT
#define F_CURSOR_TO_XMLSCHEMA DATAINSERT
#define F_QUERY_TO_XML DATAINSERT
#define F_QUERY_TO_XML_AND_XMLSCHEMA DATAINSERT
#define F_QUERY_TO_XMLSCHEMA DATAINSERT
#define F_TABLE_TO_XML DATAINSERT
#define F_TABLE_TO_XML_AND_XMLSCHEMA DATAINSERT
#define F_TABLE_TO_XMLSCHEMA DATAINSERT
#define F_BYTEAOUT 31
#define F_CHAROUT 33

const FmgrBuiltin fmgr_builtins[] = {
{ 0, "cursor_to_xml", 5, true, false, cursor_to_xml },
{ 0, "cursor_to_xmlschema", 4, true, false, cursor_to_xmlschema },
{ 0, "query_to_xml", 4, true, false, query_to_xml },
{ 0, "query_to_xml_and_xmlschema", 4, true, false, query_to_xml_and_xmlschema },
{ 0, "query_to_xmlschema", 4, true, false, query_to_xmlschema },
{ 0, "table_to_xml", 4, true, false, table_to_xml },
{ 0, "table_to_xml_and_xmlschema", 4, true, false, table_to_xml_and_xmlschema },
{ 0, "table_to_xmlschema", 4, true, false, table_to_xmlschema },
{ 31, "byteaout", 1, true, false, byteaout },

The fact that that table is broken means you're incurring expensive
linear searches to invoke these functions. It's only by chance that
it works at all...

In the second place, if you don't want to predetermine OIDs for your
functions then they shouldn't be in hardwired pg_proc.h rows at all.

regards, tom lane

#7Gregory Stark
stark@enterprisedb.com
In reply to: Tom Lane (#6)
Re: pg_proc without oid?

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

In the second place, if you don't want to predetermine OIDs for your
functions then they shouldn't be in hardwired pg_proc.h rows at all.

Is there any place to hook in to create things like procedures or other SQL
objects that don't really need hard coded OIDs?

It seems like we could make the catalogs much easier to maintain by ripping
out everything that isn't needed by the system tables themselves and having
initdb create them by running a plain SQL script.

In particular I'm looking towards having all the operators and associated
hardware except for the basic btree operators for types used by the system
tables be created in plain SQL.

It might also reduce the pain OID conflicts cause.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

#8Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#6)
Re: pg_proc without oid?

Am Montag, 19. Februar 2007 16:50 schrieb Tom Lane:

Well, in the first place Gen_fmgrtab.sh is producing garbage:

Uh, ok, that needs fixing.

In the second place, if you don't want to predetermine OIDs for your
functions then they shouldn't be in hardwired pg_proc.h rows at all.

Where else would you put them?

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

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#8)
Re: pg_proc without oid?

Peter Eisentraut <peter_e@gmx.net> writes:

Am Montag, 19. Februar 2007 16:50 schrieb Tom Lane:

In the second place, if you don't want to predetermine OIDs for your
functions then they shouldn't be in hardwired pg_proc.h rows at all.

Where else would you put them?

SQL script maybe, much along the lines Greg was just mentioning.
(I'd been thinking myself earlier that pg_amop/amproc/etc would be a
whole lot easier to maintain if we could feed CREATE OPERATOR CLASS
commands to the bootstrap process.) But getting there will take
nontrivial work; you can't just decide to leave out a few OIDs on the
spur of the moment.

Magnus, I'd suggest reverting whatever you did to your MSVC script,
so we'll find out the next time someone makes this mistake...

regards, tom lane

#10Alvaro Herrera
alvherre@commandprompt.com
In reply to: Peter Eisentraut (#8)
Re: pg_proc without oid?

Peter Eisentraut wrote:

Am Montag, 19. Februar 2007 16:50 schrieb Tom Lane:

Well, in the first place Gen_fmgrtab.sh is producing garbage:

Uh, ok, that needs fixing.

In the second place, if you don't want to predetermine OIDs for your
functions then they shouldn't be in hardwired pg_proc.h rows at all.

Where else would you put them?

But _why_ wouldn't you want to have fixed OIDs for the functions? I'm
not seeing the benefit.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#11Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#9)
Re: pg_proc without oid?

Tom Lane wrote:

SQL script maybe, much along the lines Greg was just mentioning.

I would welcome that, although a similar suggestion was rejected a few
years ago, which is why I didn't pursue it here.

you can't just decide to leave out a few OIDs on the
spur of the moment.

I still don't understand why that would be a problem, aside from the
fmgrtab problem that is specific to pg_proc. Other system catalogs
also have mixed entries with and without explicit OIDs.

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

#12Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#9)
Re: pg_proc without oid?

On Mon, Feb 19, 2007 at 11:18:36AM -0500, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Am Montag, 19. Februar 2007 16:50 schrieb Tom Lane:

In the second place, if you don't want to predetermine OIDs for your
functions then they shouldn't be in hardwired pg_proc.h rows at all.

Where else would you put them?

SQL script maybe, much along the lines Greg was just mentioning.
(I'd been thinking myself earlier that pg_amop/amproc/etc would be a
whole lot easier to maintain if we could feed CREATE OPERATOR CLASS
commands to the bootstrap process.) But getting there will take
nontrivial work; you can't just decide to leave out a few OIDs on the
spur of the moment.

Magnus, I'd suggest reverting whatever you did to your MSVC script,
so we'll find out the next time someone makes this mistake...

Ok. Will do once the entires in pg_proc are changed, so that I can still
build.

BTW, another problem with the stuff that's in there now - pg_proc.h
contains description entries for the functions, but that never goes in
to pg_description, since there is no oid to bind it to...

//Magnus

#13Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#12)
Re: pg_proc without oid?

Am Dienstag, 20. Februar 2007 09:24 schrieb Magnus Hagander:

Ok. Will do once the entires in pg_proc are changed, so that I can still
build.

It's done.

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

#14Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#9)
Re: pg_proc without oid?

On Mon, Feb 19, 2007 at 11:18:36AM -0500, Tom Lane wrote:

Magnus, I'd suggest reverting whatever you did to your MSVC script,
so we'll find out the next time someone makes this mistake...

Reverted. I left the part in genbki.pl in there, because that's a plain
bug that was exposed by this, and could at least theoretically be
exposed in other ways as well.

//Magnus