Proposal: variant of regclass

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I would like to add a variant of regclass, which is exactly same as
current regclass except it does not raise an error when the target
table is not found. Instead it returns InvalidOid (0).

I've sometimes thought we should just make all the reg* input converters
act that way. It's not terribly consistent that they'll happily take
numeric inputs that don't correspond to any existing OID. And more
often than not, I've found the throw-an-error behavior to be annoying
not helpful.

In any case, -1 for dealing with this only for regclass and not the
other ones.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I would like to add a variant of regclass, which is exactly same as
current regclass except it does not raise an error when the target
table is not found. Instead it returns InvalidOid (0).

I've sometimes thought we should just make all the reg* input converters
act that way. It's not terribly consistent that they'll happily take
numeric inputs that don't correspond to any existing OID. And more
often than not, I've found the throw-an-error behavior to be annoying
not helpful.

In any case, -1 for dealing with this only for regclass and not the
other ones.

I'm happy with changing reg* at the same time. Will come up with the
modified proposal.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello, Tom.

You wrote:

TL> Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I would like to add a variant of regclass, which is exactly same as
current regclass except it does not raise an error when the target
table is not found. Instead it returns InvalidOid (0).

TL> I've sometimes thought we should just make all the reg* input converters
TL> act that way.

Absolutely agree. I cannot see the case whn error is the appropriate
solution. Casting nonexistent objects to NULL is the way to go for me.

TL> It's not terribly consistent that they'll happily take
TL> numeric inputs that don't correspond to any existing OID. And more
TL> often than not, I've found the throw-an-error behavior to be annoying
TL> not helpful.

TL> In any case, -1 for dealing with this only for regclass and not the
TL> other ones.

TL> regards, tom lane

--
With best wishes,
Pavel mailto:pavel@gf.microolap.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2013-12-04 20:25:53 -0500, Tom Lane wrote:

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I would like to add a variant of regclass, which is exactly same as
current regclass except it does not raise an error when the target
table is not found. Instead it returns InvalidOid (0).

I've sometimes thought we should just make all the reg* input converters
act that way. It's not terribly consistent that they'll happily take
numeric inputs that don't correspond to any existing OID. And more
often than not, I've found the throw-an-error behavior to be annoying
not helpful.

I find that to be a bit of a scary change. I have seen application check
for the existance of tables using the error thrown by ::regclass. Now,
they could change that to check for IS NULL which would be better for
them performancewise, but the likelihood they will notice in time seems
small.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2013/12/5 Andres Freund <andres@2ndquadrant.com>

On 2013-12-04 20:25:53 -0500, Tom Lane wrote:

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I would like to add a variant of regclass, which is exactly same as
current regclass except it does not raise an error when the target
table is not found. Instead it returns InvalidOid (0).

I've sometimes thought we should just make all the reg* input converters
act that way. It's not terribly consistent that they'll happily take
numeric inputs that don't correspond to any existing OID. And more
often than not, I've found the throw-an-error behavior to be annoying
not helpful.

I find that to be a bit of a scary change. I have seen application check
for the existance of tables using the error thrown by ::regclass. Now,
they could change that to check for IS NULL which would be better for
them performancewise, but the likelihood they will notice in time seems
small.

this change can break some applications

but personally I like this change.

We can introduce some assert polymorphic function

CREATE OR REPLACE FUNCTION notnull(any, message text) RETURNS any, that can
be used for check inside SQL

Regards

Pavel

On 2013-12-05 11:54:20 +0100, Pavel Stehule wrote:

2013/12/5 Andres Freund <andres@2ndquadrant.com>
We can introduce some assert polymorphic function

CREATE OR REPLACE FUNCTION notnull(any, message text) RETURNS any, that can
be used for check inside SQL

Uh. How is that going to help applications that upgraded, without having
noticed a pretty obscure notice in the release notes?

If this were day one, I would agree we should go that way, but today...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2013/12/5 Andres Freund <andres@2ndquadrant.com>

On 2013-12-05 11:54:20 +0100, Pavel Stehule wrote:

2013/12/5 Andres Freund <andres@2ndquadrant.com>
We can introduce some assert polymorphic function

CREATE OR REPLACE FUNCTION notnull(any, message text) RETURNS any, that

can

be used for check inside SQL

Uh. How is that going to help applications that upgraded, without having
noticed a pretty obscure notice in the release notes?

this function doesn't replace a "obscure notice in the release notes".

On second hand is better to throw unpractically designed feature early
than hold it forever.

If there was not too aversion against GUC, I can say, so for some time GUC
can be solution. But it isnot

Regards

Pavel

Hello, Andres.

You wrote:

AF> On 2013-12-04 20:25:53 -0500, Tom Lane wrote:

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I would like to add a variant of regclass, which is exactly same as
current regclass except it does not raise an error when the target
table is not found. Instead it returns InvalidOid (0).

I've sometimes thought we should just make all the reg* input converters
act that way. It's not terribly consistent that they'll happily take
numeric inputs that don't correspond to any existing OID. And more
often than not, I've found the throw-an-error behavior to be annoying
not helpful.

AF> I find that to be a bit of a scary change. I have seen application check
AF> for the existance of tables using the error thrown by ::regclass. Now,
AF> they could change that to check for IS NULL which would be better for
AF> them performancewise, but the likelihood they will notice in time seems
AF> small.

I personally see two approaches:
1. Implement GUC variable controling this behaviour per session
2. Introduce new safe reg* variables, e.g. "sregclass", "sregtype" etc.

AF> Greetings,

AF> Andres Freund

AF> --
AF> Andres Freund http://www.2ndQuadrant.com/
AF> PostgreSQL Development, 24x7 Support, Training & Services

--
With best wishes,
Pavel mailto:pavel@gf.microolap.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Pavel Golub <pavel@microolap.com> writes:

I personally see two approaches:
1. Implement GUC variable controling this behaviour per session
2. Introduce new safe reg* variables, e.g. "sregclass", "sregtype" etc.

I don't think new types are a good idea. If we are afraid to change
the behavior of the input converters, what we should do is introduce
new functions, eg "toregclass(text) returns regclass".

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 12/5/13, 9:41 AM, Tom Lane wrote:

Pavel Golub <pavel@microolap.com> writes:

I personally see two approaches:
1. Implement GUC variable controling this behaviour per session
2. Introduce new safe reg* variables, e.g. "sregclass", "sregtype" etc.

I don't think new types are a good idea. If we are afraid to change
the behavior of the input converters, what we should do is introduce
new functions, eg "toregclass(text) returns regclass".

We could invent some sneaky syntax variants, like 'pg_klass'::regclass
errors, but '?pg_klass'::regclass does not.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:

We could invent some sneaky syntax variants, like 'pg_klass'::regclass
errors, but '?pg_klass'::regclass does not.

Hmm ... cute idea, but shoehorning it into regoperator might be
problematic. You'd have to pick a flag character that wasn't a
valid operator character, which lets out '?' as well as a lot
of the other mnemonically-reasonable choices.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Dec 5, 2013 at 9:41 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Pavel Golub <pavel@microolap.com> writes:

I personally see two approaches:
1. Implement GUC variable controling this behaviour per session
2. Introduce new safe reg* variables, e.g. "sregclass", "sregtype" etc.

I don't think new types are a good idea. If we are afraid to change
the behavior of the input converters, what we should do is introduce
new functions, eg "toregclass(text) returns regclass".

That seems like a pretty reasonable approach.

I don't have a strong opinion on whether it's worth the
backward-compatibility break that would ensue from just changing this
outright. I admit that I've been annoyed by this behavior more than
once, but I've also been beaten by customers enough times to know that
backward-compatibility has value to other people in some cases where
it does not have such value to me.

Another advantage of this approach is that, IIUC, type input functions
can't return a NULL value. So 'pg_klass'::regclass could return 0,
but not NULL. On the other hand, toregclass('pg_klass') *could*
return NULL, which seems conceptually cleaner.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Dec 5, 2013 at 9:41 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't think new types are a good idea. If we are afraid to change
the behavior of the input converters, what we should do is introduce
new functions, eg "toregclass(text) returns regclass".

That seems like a pretty reasonable approach.

I don't have a strong opinion on whether it's worth the
backward-compatibility break that would ensue from just changing this
outright. I admit that I've been annoyed by this behavior more than
once, but I've also been beaten by customers enough times to know that
backward-compatibility has value to other people in some cases where
it does not have such value to me.

I'm getting less enamored of just-change-the-input-behavior myself.
The case that occurred to me is, suppose somebody's got a table containing
a regclass or regproc column, and he dumps and reloads it. If the input
converter silently replaces unknown names by 0, he's at risk of unexpected
data loss, if the reload is done before he's created all the referenced
objects.

Another advantage of this approach is that, IIUC, type input functions
can't return a NULL value. So 'pg_klass'::regclass could return 0,
but not NULL. On the other hand, toregclass('pg_klass') *could*
return NULL, which seems conceptually cleaner.

Yeah, I was thinking of that too.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

Another advantage of this approach is that, IIUC, type input functions
can't return a NULL value. So 'pg_klass'::regclass could return 0,
but not NULL. On the other hand, toregclass('pg_klass') *could*
return NULL, which seems conceptually cleaner.

BTW, another arguable advantage of fixing this via new functions is
that users could write equivalent (though no doubt slower) functions
for use in pre-9.4 releases, and thus not need to maintain multiple
versions of app code that relies on this behavior.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 12/5/13, 10:08 AM, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

We could invent some sneaky syntax variants, like 'pg_klass'::regclass
errors, but '?pg_klass'::regclass does not.

Hmm ... cute idea, but shoehorning it into regoperator might be
problematic. You'd have to pick a flag character that wasn't a
valid operator character, which lets out '?' as well as a lot
of the other mnemonically-reasonable choices.

Well, you could pick any letter, I suppose.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Dec 5, 2013, at 7:52 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

BTW, another arguable advantage of fixing this via new functions is
that users could write equivalent (though no doubt slower) functions
for use in pre-9.4 releases, and thus not need to maintain multiple
versions of app code that relies on this behavior.

+1 to this idea. Feels cleanest.

David

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I'm getting less enamored of just-change-the-input-behavior myself.
The case that occurred to me is, suppose somebody's got a table containing
a regclass or regproc column, and he dumps and reloads it. If the input
converter silently replaces unknown names by 0, he's at risk of unexpected
data loss, if the reload is done before he's created all the referenced
objects.

Another advantage of this approach is that, IIUC, type input functions
can't return a NULL value. So 'pg_klass'::regclass could return 0,
but not NULL. On the other hand, toregclass('pg_klass') *could*
return NULL, which seems conceptually cleaner.

Yeah, I was thinking of that too.

Can I make sure that we want to keep the current behavior:

test=# SELECT 'pg_klass'::regclass;
ERROR: relation "pg_klass" does not exist
LINE 1: SELECT 'pg_klass'::regclass;
^

Or do we want the SELECT to return 0 in the case above?
I'm asking because of this:

So 'pg_klass'::regclass could return 0,
but not NULL.

In the mean time I agree the idea that we add:
toregclass(text) returns regclass
and friends.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tatsuo Ishii <ishii@postgresql.org> writes:

Can I make sure that we want to keep the current behavior:

test=# SELECT 'pg_klass'::regclass;
ERROR: relation "pg_klass" does not exist

Yeah, I think the consensus is to not change the behavior of the input
functions, just add some new ones.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tatsuo Ishii <ishii@postgresql.org> writes:

Can I make sure that we want to keep the current behavior:

test=# SELECT 'pg_klass'::regclass;
ERROR: relation "pg_klass" does not exist

Yeah, I think the consensus is to not change the behavior of the input
functions, just add some new ones.

Ok, here is the conceptual patch to implement "toregclass" (only for
now). If my direction is ok, I'll come up with complete patches to
implement more "to*" functions. Any advice will be appreciated.

Here is a sample session:

test=# select toregclass('foo');
toregclass
------------
-
(1 row)

test=# select toregclass('pg_class');
toregclass
------------
pg_class
(1 row)

test=# select toregclass('pg_class')::oid;
toregclass
------------
1259
(1 row)

test=# select toregclass('foo')::oid;
toregclass
------------
0
(1 row)

Implementation notes:

To implement toregclass, which does not throw errors when invalid
argument is given, src/backend/utils/adt/regproc.c is modified. I
added two static functions:

static Datum regclass_gut(char *class_name_or_oid, bool raiseError);
static List *stringToQualifiedNameList_gut(const char *string, bool raiseError);

regclass_gut is called from regclassin and toregclass and do the most
job before regclassin did. "raiseError" flag controls whether an error
is raised or not when an invalid argument (for example non existent
relation) is given. For this purpose, regclass_gut wraps the call to
oidin using a PG_TRY block.

Secondly, when called as bootstap and raiseError is true, returns
InvalidOid instead of raising an error "relation XXX does not
exist". However, I doubt there's no customer who calls regclass_gut
with raiseError is false in the bootstrap.

Thirdly, stringToQualifiedNameList_gut is added to replace
stringToQualifiedNameList. The reason why I don't use PG_TRY block is,
I need to free some memory allocated inside the function in an
error condition.

Finially I modified the call to RangeVarGetRelid to switch
"missing_ok" flag to reflect raiseError argument.

One thing I need to further is modifying makeRangeVarFromNameList. If
strange schema qualified name like "a.b.c.d.e.f" is given, still an
error raises.

So, any advice will be appreciated.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp