Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, I fonund that pg_proc.h got modified so rebased and
rearranged the patchset merging the recent fixes.

regards,

 I sent the previous mail unfinished.
 
 At Thu, 09 Apr 2015 17:25:10 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 horiguchi.kyot...@lab.ntt.co.jp wrote in 
 20150409.172510.29010318.horiguchi.kyot...@lab.ntt.co.jp
  Hello, sorry for the absence. I changed the regnamespace's
  behavior as the same as the other reg* types. And I attached a
  patch as separate one that fixes regroleout to do the same as the
  other reg* types, because I have
 
 because, I doubt that it is appropriate way to do so.
 
  0001-Add-regrole_v6.patch : fix regnamespace to behave as the
  same as the other reg* types.
  
  0001-Make-regnamespace-behave-as-the-same-as-other-reg-ty.patch:
Diff from v5 to v6, only for convinience.
  
  0001-Fix-regroleout-s-behavior-following-other-out-functi.patch:
Fixes regroleout so as to behave as the same as other reg*
types, but might be a bit too large.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 2210bc524906ec1c9fdf4649260568b0ba807c30 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp
Date: Wed, 18 Feb 2015 14:38:32 +0900
Subject: [PATCH 1/2] Add regrole-v7

Add new OID aliass type regrole. The new type has the scope of whole
the database cluster so it doesn't behave as the same as the existing
OID alias types which have database scope, concerning object
dependency. To get rid of confusion, inhibit constants of the new type
from appearing where dependencies are made involving it.

Documentation about this new type is added and some existing
descriptions are modified according to the restriction of the
type. Addition to it, put a note about possible MVCC violation and
optimization issues, which are general over the all reg* types.
---
 contrib/spi/insert_username.c |   2 +-
 contrib/spi/timetravel.c  |   2 +-
 doc/src/sgml/datatype.sgml|  27 +++--
 src/backend/bootstrap/bootstrap.c |   2 +
 src/backend/catalog/dependency.c  |  10 
 src/backend/catalog/objectaddress.c   |  20 +++
 src/backend/utils/adt/acl.c   |   2 +-
 src/backend/utils/adt/name.c  |   4 +-
 src/backend/utils/adt/regproc.c   | 104 ++
 src/backend/utils/adt/selfuncs.c  |   2 +
 src/backend/utils/cache/catcache.c|   1 +
 src/backend/utils/init/miscinit.c |  24 +---
 src/include/catalog/pg_cast.h |   7 +++
 src/include/catalog/pg_proc.h |  12 
 src/include/catalog/pg_type.h |   5 ++
 src/include/foreign/foreign.h |   2 +-
 src/include/miscadmin.h   |   2 +-
 src/include/utils/builtins.h  |   5 ++
 src/test/regress/expected/regproc.out |  26 -
 src/test/regress/sql/regproc.sql  |   7 +++
 20 files changed, 235 insertions(+), 31 deletions(-)

diff --git a/contrib/spi/insert_username.c b/contrib/spi/insert_username.c
index 8752078..3812525 100644
--- a/contrib/spi/insert_username.c
+++ b/contrib/spi/insert_username.c
@@ -79,7 +79,7 @@ insert_username(PG_FUNCTION_ARGS)
 		args[0], relname)));
 
 	/* create fields containing name */
-	newval = CStringGetTextDatum(GetUserNameFromId(GetUserId()));
+	newval = CStringGetTextDatum(GetUserNameFromId(GetUserId(), false));
 
 	/* construct new tuple */
 	rettuple = SPI_modifytuple(rel, rettuple, 1, attnum, newval, NULL);
diff --git a/contrib/spi/timetravel.c b/contrib/spi/timetravel.c
index 0699438..e125986 100644
--- a/contrib/spi/timetravel.c
+++ b/contrib/spi/timetravel.c
@@ -174,7 +174,7 @@ timetravel(PG_FUNCTION_ARGS)
 	}
 
 	/* create fields containing name */
-	newuser = CStringGetTextDatum(GetUserNameFromId(GetUserId()));
+	newuser = CStringGetTextDatum(GetUserNameFromId(GetUserId(), false));
 
 	nulltext = (Datum) NULL;
 
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index da1f25f..0cac993 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4321,8 +4321,9 @@ SET xmloption TO { DOCUMENT | CONTENT };
 an object identifier.  There are also several alias types for
 typeoid/: typeregproc/, typeregprocedure/,
 typeregoper/, typeregoperator/, typeregclass/,
-typeregtype/, typeregconfig/, and typeregdictionary/.
-xref linkend=datatype-oid-table shows an overview.
+typeregtype/, typeregrole/, typeregconfig/, and
+typeregdictionary/.  xref linkend=datatype-oid-table shows
+an overview.
/para
 
para
@@ -4431,6 +4432,13 @@ SELECT * FROM pg_attribute
/row
 
row
+entrytyperegrole//entry
+entrystructnamepg_authid//entry
+entryrole name/entry
+entryliteralsmithee//entry
+   /row
+
+   row
 entrytyperegconfig//entry
 entrystructnamepg_ts_config//entry
 entrytext search configuration/entry
@@ -4448,7 +4456,8 @@ SELECT * FROM pg_attribute
 /table
 
para
-

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, sorry for the absence. I changed the regnamespace's
behavior as the same as the other reg* types. And I attached a
patch as separate one that fixes regroleout to do the same as the
other reg* types, because I have

0001-Add-regrole_v6.patch : fix regnamespace to behave as the
same as the other reg* types.

0001-Make-regnamespace-behave-as-the-same-as-other-reg-ty.patch:
  Diff from v5 to v6, only for convinience.

0001-Fix-regroleout-s-behavior-following-other-out-functi.patch:
  Fixes regroleout so as to behave as the same as other reg*
  types, but might be a bit too large.


At Wed, 01 Apr 2015 14:46:01 -0400, Andrew Dunstan and...@dunslane.net wrote 
in 551c3ce9.4080...@dunslane.net
  But in any case I cannot see a reason to treat regnamespace
  differently
  from the existing types on this point.
 
  
 
 Good points.
 
 I agree re namespace. And I also agree that shared dependency support
 is not worth the trouble, especially not just to support regrole. I'm
 not sure that's a reason to reject regrole entirely, though. However,
 I also think there is a significantly less compelling case for it than
 for regnamespace, based on the number of times I have wanted each.
 
 Anybody else have thoughts on this?

Yeah, that's my thinko. regnamespace in the attached patch
behaves as the same to other reg* types. On the way fixing it, I
found a bug that regnamespaceout returns NULL for removed shema
name. I fixed it, too.

Addition to that, regroleout raises exception for invalid role
oids. This is unwanted behavior but GetUserNameFromId is needed
to have one extra parameter 'noerr' to fix this. This fix is
attached as another patch.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index da1f25f..6d1f02d 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4321,7 +4321,8 @@ SET xmloption TO { DOCUMENT | CONTENT };
 an object identifier.  There are also several alias types for
 typeoid/: typeregproc/, typeregprocedure/,
 typeregoper/, typeregoperator/, typeregclass/,
-typeregtype/, typeregconfig/, and typeregdictionary/.
+typeregtype/, typeregrole/, typeregnamespace/, 
+typeregconfig/, and typeregdictionary/.
 xref linkend=datatype-oid-table shows an overview.
/para
 
@@ -4431,6 +4432,20 @@ SELECT * FROM pg_attribute
/row
 
row
+entrytyperegrole//entry
+entrystructnamepg_authid//entry
+entryrole name/entry
+entryliteralsmithee//entry
+   /row
+
+   row
+entrytyperegnamespace//entry
+entrystructnamepg_namespace//entry
+entrynamespace name/entry
+entryliteralpg_catalog//entry
+   /row
+
+   row
 entrytyperegconfig//entry
 entrystructnamepg_ts_config//entry
 entrytext search configuration/entry
@@ -4448,29 +4463,37 @@ SELECT * FROM pg_attribute
 /table
 
para
-All of the OID alias types accept schema-qualified names, and will
-display schema-qualified names on output if the object would not
-be found in the current search path without being qualified.
-The typeregproc/ and typeregoper/ alias types will only
-accept input names that are unique (not overloaded), so they are
-of limited use; for most uses typeregprocedure/ or
+All of the OID alias types for objects grouped by namespace accept
+schema-qualified names, and will display schema-qualified names on output
+if the object would not be found in the current search path without being
+qualified.  The typeregproc/ and typeregoper/ alias types will
+only accept input names that are unique (not overloaded), so they are of
+limited use; for most uses typeregprocedure/ or
 typeregoperator/ are more appropriate.  For typeregoperator/,
 unary operators are identified by writing literalNONE/ for the unused
 operand.
/para
 
+   para An additional property of most of the OID alias types is the
+creation of dependencies.  If a constant of one of these types
+appears in a stored expression (such as a column default
+expression or view), it creates a dependency on the referenced
+object.  For example, if a column has a default expression
+literalnextval('my_seq'::regclass)/,
+productnamePostgreSQL/productname understands that the default
+expression depends on the sequence literalmy_seq/; the system
+will not let the sequence be dropped without first removing the
+default expression. typeregrole/ is the only exception for the
+property. Constants of this type is not allowed in such
+expressions.  /para
+
+   note
para
-An additional property of the OID alias types is the creation of
-dependencies.  If a
-constant of one of these types appears in a stored expression
-(such as a column default expression or view), it creates a dependency
-on the 

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

I sent the previous mail unfinished.

At Thu, 09 Apr 2015 17:25:10 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
horiguchi.kyot...@lab.ntt.co.jp wrote in 
20150409.172510.29010318.horiguchi.kyot...@lab.ntt.co.jp
 Hello, sorry for the absence. I changed the regnamespace's
 behavior as the same as the other reg* types. And I attached a
 patch as separate one that fixes regroleout to do the same as the
 other reg* types, because I have

because, I doubt that it is appropriate way to do so.

 0001-Add-regrole_v6.patch : fix regnamespace to behave as the
 same as the other reg* types.
 
 0001-Make-regnamespace-behave-as-the-same-as-other-reg-ty.patch:
   Diff from v5 to v6, only for convinience.
 
 0001-Fix-regroleout-s-behavior-following-other-out-functi.patch:
   Fixes regroleout so as to behave as the same as other reg*
   types, but might be a bit too large.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Robert Haas]

On Sat, Mar 28, 2015 at 6:59 PM, Andrew Dunstan and...@dunslane.net wrote:
 I have just claimed this as committer in the CF, but on reviewing the emails
 it looks like there is disagreement about the need for it at all, especially
 from Tom and Robert.

 I confess I have often wanted regnamespace, particularly, and occasionally
 regrole, simply as a convenience. But I'm not going to commit it against
 substantial opposition.

 Do we need a vote?

Seeing this committed this wouldn't be my first choice, but I can live
with it, as long as the patch is good technically.  As useful as these
sorts of types are, I'm not convinced that notational convenience for
people steeped in backend internals is a sufficiently-good reason to
clutter the system with more built-in types.  But I'm probably in the
minority on that; and it's clearly a judgement call anyway.

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

Re: [HACKERS] How about to have relnamespace and relrole?

[Andrew Dunstan]

On 03/31/2015 04:48 PM, Tom Lane wrote:


In view of that, you could certainly argue that if someone's bothered
to make a patch to add a new regFOO type, it's useful enough.  I don't
want to end up with thirtysomething of them, but we don't seem to be
trending in that direction.

Or in short, objection withdrawn.  (As to the concept, anyway.
I've not read the patch...)






The only possible issue I see on reading the patches is that these are 
treated differently for dependencies than other regFOO types. Rather 
than create a dependency if a value is used in a default expression, an 
error is raised if one is found. Are we OK with that?


cheers

andrew


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Tom Lane]

Andrew Dunstan and...@dunslane.net writes:
 The only possible issue I see on reading the patches is that these are 
 treated differently for dependencies than other regFOO types. Rather 
 than create a dependency if a value is used in a default expression, an 
 error is raised if one is found. Are we OK with that?

Why would it be a good idea to act differently from the others?

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

Re: [HACKERS] How about to have relnamespace and relrole?

[Andrew Dunstan]

On 04/01/2015 12:53 PM, Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:

On 04/01/2015 12:13 PM, Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:

The only possible issue I see on reading the patches is that these are
treated differently for dependencies than other regFOO types. Rather
than create a dependency if a value is used in a default expression, an
error is raised if one is found. Are we OK with that?

Why would it be a good idea to act differently from the others?

I have no idea.
It was mentioned here
http://www.postgresql.org/message-id/20150218.174231.125293096.horiguchi.kyot...@lab.ntt.co.jp
but nobody seems to have commented. I'm not sure why it was done like
this. Adding the dependencies seems to be no harder than raising the
exception. I think we can kick this back to the author to fix.

After a bit more thought it occurred to me that a dependency on a role
would need to be a shared dependency, and the existing infrastructure
for recordDependencyOnExpr() wouldn't support that.

I'm not sure that it's worth adding the complexity to allow shared
dependencies along with normal ones there.  This might be a reason
to reject the regrole part of the patch, as requiring more complexity
than it's worth.

But in any case I cannot see a reason to treat regnamespace differently
from the existing types on this point.




Good points.

I agree re namespace. And I also agree that shared dependency support is 
not worth the trouble, especially not just to support regrole. I'm  not 
sure that's a reason to reject regrole entirely, though. However, I also 
think there is a significantly less compelling case for it than for 
regnamespace, based on the number of times I have wanted each.


Anybody else have thoughts on this?

cheers

andrew


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Andrew Dunstan]

On 04/01/2015 12:13 PM, Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:

The only possible issue I see on reading the patches is that these are
treated differently for dependencies than other regFOO types. Rather
than create a dependency if a value is used in a default expression, an
error is raised if one is found. Are we OK with that?

Why would it be a good idea to act differently from the others?




I have no idea.

It was mentioned here 
http://www.postgresql.org/message-id/20150218.174231.125293096.horiguchi.kyot...@lab.ntt.co.jp 
but nobody seems to have commented. I'm not sure why it was done like 
this. Adding the dependencies seems to be no harder than raising the 
exception. I think we can kick this back to the author to fix.


cheers

andrew






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

Re: [HACKERS] How about to have relnamespace and relrole?

[Tom Lane]

Andrew Dunstan and...@dunslane.net writes:
 On 04/01/2015 12:13 PM, Tom Lane wrote:
 Andrew Dunstan and...@dunslane.net writes:
 The only possible issue I see on reading the patches is that these are
 treated differently for dependencies than other regFOO types. Rather
 than create a dependency if a value is used in a default expression, an
 error is raised if one is found. Are we OK with that?

 Why would it be a good idea to act differently from the others?

 I have no idea.

 It was mentioned here 
 http://www.postgresql.org/message-id/20150218.174231.125293096.horiguchi.kyot...@lab.ntt.co.jp
  
 but nobody seems to have commented. I'm not sure why it was done like 
 this. Adding the dependencies seems to be no harder than raising the 
 exception. I think we can kick this back to the author to fix.

After a bit more thought it occurred to me that a dependency on a role
would need to be a shared dependency, and the existing infrastructure
for recordDependencyOnExpr() wouldn't support that.

I'm not sure that it's worth adding the complexity to allow shared
dependencies along with normal ones there.  This might be a reason
to reject the regrole part of the patch, as requiring more complexity
than it's worth.

But in any case I cannot see a reason to treat regnamespace differently
from the existing types on this point.

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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello,

At Tue, 31 Mar 2015 16:48:18 -0400, Tom Lane t...@sss.pgh.pa.us wrote in 
26969.1427834...@sss.pgh.pa.us
 Hmm.  We can ignore pg_attribute and pg_pltemplate, which don't have OIDs
 and thus aren't candidates anyway.  And we can ignore the ones
 corresponding to the already-existing regFOO types.  That leaves
 
pg_am  | amname
pg_authid  | rolname  (*)
pg_collation   | collname
pg_constraint  | conname
pg_conversion  | conname
pg_database| datname
pg_event_trigger   | evtname
pg_extension   | extname
pg_foreign_data_wrapper| fdwname
pg_foreign_server  | srvname
pg_language| lanname
pg_namespace   | nspname  (*)
pg_opclass | opcname
pg_opfamily| opfname
pg_policy  | polname
pg_rewrite | rulename
pg_tablespace  | spcname
pg_trigger | tgname
pg_ts_parser   | prsname
pg_ts_template | tmplname
 
 of which the proposed patch covers the two starred ones.
 
 OTOH, looking at this list, there are already numerous cases where
 the object identity is more than just a name (eg, collations have
 schema-qualified names, opfamilies are not only schema-qualified
 but are per-index-AM as well, triggers and constraints are named
 per-table, etc).  So it's clear that we've already been applying
 a usefulness criterion rather than just does it have a
 multi-component name when choosing which objects to provide
 regFOO types for.

As I wrote before, the criteria I selected for choosing these
ones was how often the oid is referred to. The attached excel
file shows the complehensive list of reference counts.

Each cells is marked 'x' if the catalog of the row referrs to the
oid of the catalog on the column. So the numbers in the row 2
represents how mane times the oid of the catalog on the column is
referred to from other catalogs. Adding all catalog having tuple
oid and sorting by the number they are ordered as below.

(The upper cased 'X' in the HASOID column indicates that the view
 exposes the oid of underlying table and identifying the rows in
 the view)

(-) in the list below is the regFOO types already exists and the
second column is the number of other catalogs refers to the oid.

   pg_authid | 33 | rolname  (*)
+   pg_class  | 27 | relname  (-)
   pg_namespace  | 20 | nspname  (*)
+   pg_type   | 15 | typname  (-)
+   pg_proc   | 13 | proname  (-)
+   pg_operator   |  5 | oprname  (-)
   pg_database   |  5 | datname
   pg_am |  4 | amname
   pg_collation  |  4 | collname
   pg_tablespace |  4 | spcname
   pg_foreign_server |  3 | srvname
   pg_opfamily   |  3 | opfname
   pg_opclass|  2 | opcname
   pg_constraint |  1 | conname
   pg_foreign_data_wrapper   |  1 | fdwname
   pg_language   |  1 | lanname
+   pg_largeobject_metadata   |  1 | -
   pg_policy |  1 | polname
   pg_rewrite|  1 | rulename
+   pg_ts_config  |  1 | cfgname  (-)
+   pg_ts_dict|  1 | dictname (-)
   pg_ts_parser  |  1 | prsname
   pg_ts_template|  1 | tmplname
+   pg_user_mapping   |  1 | -
+   pg_aggregate  |  0 | - 

All of amop, amproc, attrdef, cast, conversion, default_acl,
enum, event_trigger, extension, group, roles, shadow, trigger,
user are not referred to from any other catalogs.

 In view of that, you could certainly argue that if someone's bothered
 to make a patch to add a new regFOO type, it's useful enough.  I don't
 want to end up with thirtysomething of them, but we don't seem to be
 trending in that direction.

pg_authid and pg_namespace are obviously win the race but haven't
got the prize. database to tablespace are in a gray zone but I
think they need highly significant reason to have regFOO type for
themselves.

On the other hand, regconfig(pg_ts_config) and
regdictionary(pg_ts_dist) have far less significance but I don't
assert they should be removed since they are there now.

 Or in short, 

Re: [HACKERS] How about to have relnamespace and relrole?

[Andrew Dunstan]

On 03/29/2015 02:55 PM, Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:

I have just claimed this as committer in the CF, but on reviewing the
emails it looks like there is disagreement about the need for it at all,
especially from Tom and Robert.
I confess I have often wanted regnamespace, particularly, and
occasionally regrole, simply as a convenience. But I'm not going to
commit it against substantial opposition.
Do we need a vote?

My concern about it is basically that I don't see where we stop.
The existing regFOO alias types are provided for object classes which
have nontrivial naming conventions (schema qualification, overloaded
argument types, etc), so that you can't just do select ... from
catalog where objectname = 'blah'.  That doesn't apply to namespaces
or roles.  So I'm afraid that once this precedent is established,
there will be demands for regFOO for every object class we have,
and I don't want that much clutter.

It may be that these two cases are so much more useful than any other
conceivable cases that we can do them and stop, but I don't think that
argument has been made convincingly.



Well, here's a list of all the fooname attributes in the catalog, which 
I guess are the prime candidates for regfoo pseudotypes. Besides those 
we already have and the two proposed here, I'm not sure there will be 
huge demand for others - tablespace maybe, trigger doesn't seem very 
practicable, and I could just see suggestions for collation and 
conversion, but those seem pretty marginal, and that seems to be about 
it, to me.


  attrelid  | attname
   +--
 pg_proc| proname
 pg_type| typname
 pg_attribute   | attname
 pg_class   | relname
 pg_constraint  | conname
 pg_operator| oprname
 pg_opfamily| opfname
 pg_opclass | opcname
 pg_am  | amname
 pg_language| lanname
 pg_rewrite | rulename
 pg_trigger | tgname
 pg_event_trigger   | evtname
 pg_namespace   | nspname
 pg_conversion  | conname
 pg_database| datname
 pg_tablespace  | spcname
 pg_pltemplate  | tmplname
 pg_authid  | rolname
 pg_ts_config   | cfgname
 pg_ts_dict | dictname
 pg_ts_parser   | prsname
 pg_ts_template | tmplname
 pg_extension   | extname
 pg_foreign_data_wrapper| fdwname
 pg_foreign_server  | srvname
 pg_policy  | polname
 pg_collation   | collname


cheers

andrew



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Tom Lane]

Andrew Dunstan and...@dunslane.net writes:
 On 03/29/2015 02:55 PM, Tom Lane wrote:
 It may be that these two cases are so much more useful than any other
 conceivable cases that we can do them and stop, but I don't think that
 argument has been made convincingly.

 Well, here's a list of all the fooname attributes in the catalog, which 
 I guess are the prime candidates for regfoo pseudotypes. Besides those 
 we already have and the two proposed here, I'm not sure there will be 
 huge demand for others - tablespace maybe, trigger doesn't seem very 
 practicable, and I could just see suggestions for collation and 
 conversion, but those seem pretty marginal, and that seems to be about 
 it, to me.

Hmm.  We can ignore pg_attribute and pg_pltemplate, which don't have OIDs
and thus aren't candidates anyway.  And we can ignore the ones
corresponding to the already-existing regFOO types.  That leaves

   pg_am  | amname
   pg_authid  | rolname  (*)
   pg_collation   | collname
   pg_constraint  | conname
   pg_conversion  | conname
   pg_database| datname
   pg_event_trigger   | evtname
   pg_extension   | extname
   pg_foreign_data_wrapper| fdwname
   pg_foreign_server  | srvname
   pg_language| lanname
   pg_namespace   | nspname  (*)
   pg_opclass | opcname
   pg_opfamily| opfname
   pg_policy  | polname
   pg_rewrite | rulename
   pg_tablespace  | spcname
   pg_trigger | tgname
   pg_ts_parser   | prsname
   pg_ts_template | tmplname

of which the proposed patch covers the two starred ones.

OTOH, looking at this list, there are already numerous cases where
the object identity is more than just a name (eg, collations have
schema-qualified names, opfamilies are not only schema-qualified
but are per-index-AM as well, triggers and constraints are named
per-table, etc).  So it's clear that we've already been applying
a usefulness criterion rather than just does it have a
multi-component name when choosing which objects to provide
regFOO types for.

In view of that, you could certainly argue that if someone's bothered
to make a patch to add a new regFOO type, it's useful enough.  I don't
want to end up with thirtysomething of them, but we don't seem to be
trending in that direction.

Or in short, objection withdrawn.  (As to the concept, anyway.
I've not read the patch...)

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

Re: [HACKERS] How about to have relnamespace and relrole?

[Tom Lane]

Andrew Dunstan and...@dunslane.net writes:
 I have just claimed this as committer in the CF, but on reviewing the 
 emails it looks like there is disagreement about the need for it at all, 
 especially from Tom and Robert.

 I confess I have often wanted regnamespace, particularly, and 
 occasionally regrole, simply as a convenience. But I'm not going to 
 commit it against substantial opposition.

 Do we need a vote?

My concern about it is basically that I don't see where we stop.
The existing regFOO alias types are provided for object classes which
have nontrivial naming conventions (schema qualification, overloaded
argument types, etc), so that you can't just do select ... from
catalog where objectname = 'blah'.  That doesn't apply to namespaces
or roles.  So I'm afraid that once this precedent is established,
there will be demands for regFOO for every object class we have,
and I don't want that much clutter.

It may be that these two cases are so much more useful than any other
conceivable cases that we can do them and stop, but I don't think that
argument has been made convincingly.

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

Re: [HACKERS] How about to have relnamespace and relrole?

[Andrew Dunstan]

On 03/10/2015 04:42 AM, Kyotaro HORIGUCHI wrote:

Thank you for the correction.

At Wed, 4 Mar 2015 01:01:48 -0600, Jim Nasby jim.na...@bluetreble.com wrote in 
54f6addc.8030...@bluetreble.com

On 3/3/15 8:04 PM, Kyotaro HORIGUCHI wrote:

Note: The OID alias types don't sctrictly comply the transaction
   isolation rules so do not use them where exact transaction
   isolation on the values of these types has a
   significance. Likewise, since they look as simple constants to
   planner so you might get slower plans than the queries joining
   the system tables correnspond to the OID types.

Might I suggest:

Note: The OID alias types do not completely follow transaction
isolation rules. The planner also treats them as simple constants,
which may result in sub-optimal planning.

Looks far simple and enough.
The note has been replaced with your sentence in the attached patch.





I have just claimed this as committer in the CF, but on reviewing the 
emails it looks like there is disagreement about the need for it at all, 
especially from Tom and Robert.


I confess I have often wanted regnamespace, particularly, and 
occasionally regrole, simply as a convenience. But I'm not going to 
commit it against substantial opposition.


Do we need a vote?

cheers

andrew


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, Thank you for reviewing, and sorry to have overlooked
this.

# I hope the CF app to add the author as a receiver when issueing
# a mail.

regards,

At Thu, 12 Mar 2015 11:06:29 +, Jeevan Chalke jeevan.cha...@gmail.com 
wrote in 20150312110629.2540.70807.p...@coridan.postgresql.org
 The following review has been posted through the commitfest application:
 make installcheck-world:  tested, passed
 Implements feature:   tested, passed
 Spec compliant:   tested, passed
 Documentation:tested, passed
 
 Looks good. Passing it to committer.
 
 The new status of this patch is: Ready for Committer

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Alvaro Herrera]

Kyotaro HORIGUCHI wrote:

 # I hope the CF app to add the author as a receiver when issueing
 # a mail.

Moreover, it should add everyone who was in To, From, CC in the email
that the commitfest app is replying to; if the patch authors are not
listed, add them as well.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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

Re: [HACKERS] How about to have relnamespace and relrole?

[Jeevan Chalke]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

Looks good. Passing it to committer.

The new status of this patch is: Ready for Committer


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Thank you for the correction.

At Wed, 4 Mar 2015 01:01:48 -0600, Jim Nasby jim.na...@bluetreble.com wrote 
in 54f6addc.8030...@bluetreble.com
 On 3/3/15 8:04 PM, Kyotaro HORIGUCHI wrote:
  Note: The OID alias types don't sctrictly comply the transaction
 isolation rules so do not use them where exact transaction
 isolation on the values of these types has a
 significance. Likewise, since they look as simple constants to
 planner so you might get slower plans than the queries joining
 the system tables correnspond to the OID types.
 
 Might I suggest:
 
 Note: The OID alias types do not completely follow transaction
 isolation rules. The planner also treats them as simple constants,
 which may result in sub-optimal planning.

Looks far simple and enough.
The note has been replaced with your sentence in the attached patch.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From e31c326fa8e8ee294f003258233bf4be1410fdd4 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp
Date: Wed, 18 Feb 2015 14:38:32 +0900
Subject: [PATCH 1/2] Add regrole

Add new OID aliass type regrole. The new type has the scope of whole
the database cluster so it doesn't behave as the same as the existing
OID alias types which have database scope, concerning object
dependency. To get rid of confusion, inhibit constants of the new type
from appearing where dependencies are made involving it.

Documentation about this new type is added and some existing
descriptions are modified according to the restriction of the
type. Addition to it, put a note about possible MVCC violation and
optimization issues, which are general over the all reg* types.
---
 doc/src/sgml/datatype.sgml| 53 ---
 src/backend/bootstrap/bootstrap.c |  2 +
 src/backend/catalog/dependency.c  | 22 
 src/backend/utils/adt/regproc.c   | 98 +++
 src/backend/utils/adt/selfuncs.c  |  2 +
 src/backend/utils/cache/catcache.c|  1 +
 src/include/catalog/pg_cast.h |  7 +++
 src/include/catalog/pg_proc.h | 10 
 src/include/catalog/pg_type.h |  5 ++
 src/include/utils/builtins.h  |  5 ++
 src/test/regress/expected/regproc.out | 26 +-
 src/test/regress/sql/regproc.sql  |  7 +++
 12 files changed, 219 insertions(+), 19 deletions(-)

diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index edf636b..f4e82f5 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4319,8 +4319,9 @@ SET xmloption TO { DOCUMENT | CONTENT };
 an object identifier.  There are also several alias types for
 typeoid/: typeregproc/, typeregprocedure/,
 typeregoper/, typeregoperator/, typeregclass/,
-typeregtype/, typeregconfig/, and typeregdictionary/.
-xref linkend=datatype-oid-table shows an overview.
+typeregtype/, typeregrole/, typeregconfig/, and
+typeregdictionary/.  xref linkend=datatype-oid-table shows
+an overview.
/para
 
para
@@ -4429,6 +4430,13 @@ SELECT * FROM pg_attribute
/row
 
row
+entrytyperegrole//entry
+entrystructnamepg_authid//entry
+entryrole name/entry
+entryliteralsmithee//entry
+   /row
+
+   row
 entrytyperegconfig//entry
 entrystructnamepg_ts_config//entry
 entrytext search configuration/entry
@@ -4446,29 +4454,38 @@ SELECT * FROM pg_attribute
 /table
 
para
-All of the OID alias types accept schema-qualified names, and will
-display schema-qualified names on output if the object would not
-be found in the current search path without being qualified.
-The typeregproc/ and typeregoper/ alias types will only
-accept input names that are unique (not overloaded), so they are
-of limited use; for most uses typeregprocedure/ or
+All of the OID alias types for objects grouped by namespace accept
+schema-qualified names, and will display schema-qualified names on output
+if the object would not be found in the current search path without being
+qualified.  The typeregproc/ and typeregoper/ alias types will
+only accept input names that are unique (not overloaded), so they are of
+limited use; for most uses typeregprocedure/ or
 typeregoperator/ are more appropriate.  For typeregoperator/,
 unary operators are identified by writing literalNONE/ for the unused
 operand.
/para
 
para
-An additional property of the OID alias types is the creation of
-dependencies.  If a
-constant of one of these types appears in a stored expression
-(such as a column default expression or view), it creates a dependency
-on the referenced object.  For example, if a column has a default
-expression literalnextval('my_seq'::regclass)/,
-productnamePostgreSQL/productname
-understands that the default expression depends on the sequence
-literalmy_seq/; the 

Re: [HACKERS] How about to have relnamespace and relrole?

[Jeevan Chalke]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

 1.
 +#include utils/acl.h

 Can you please add it at appropriate place as #include list is an ordered
 list

regrolein calls reg_role_oid in acl.c, which is declared in acl.h.

Well. I was suggesting that putting  #include utils/acl.h line after
#include parser/parse_type.h and before #include utils/builtins.h
so that they will be in order.
I understand that it is needed for reg_role_oid() call.

Review comments on *_v4 patches:

1.
+ The OID alias types don't sctrictly comply the transaction isolation

Typo. sctrictly = strictly


2.
+ joining the system tables correnspond to the OID types.
Typo. correnspond = correspond

Apart from these typos, I see no issues.
However, you can have a look over Jim's suggestion on doc wordings.

If you feel comfortable, I have no issues if you mark this as
Ready for Committer once you fix the typos and doc wordings.


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Jim Nasby]

On 3/3/15 8:04 PM, Kyotaro HORIGUCHI wrote:

Note: The OID alias types don't sctrictly comply the transaction
   isolation rules so do not use them where exact transaction
   isolation on the values of these types has a
   significance. Likewise, since they look as simple constants to
   planner so you might get slower plans than the queries joining
   the system tables correnspond to the OID types.


Might I suggest:

Note: The OID alias types do not completely follow transaction isolation 
rules. The planner also treats them as simple constants, which may 
result in sub-optimal planning.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, I attached the latest patches missing in the previous mail.

Thanks for pointing Jeevan.

0001-Add-regrole_v4.patch
0002-Add-regnamespace_v4.patch

Jim BTW, I think the potential for MVCC issues should be mentioned in the
Jim docs (http://www.postgresql.org/docs/devel/static/datatype-oid.html).

The first patch of the aboves contains doc patch which adds the
following note to html/datatype-oid.html. Does it make sense?

 Note: The OID alias types don't sctrictly comply the transaction
   isolation rules so do not use them where exact transaction
   isolation on the values of these types has a
   significance. Likewise, since they look as simple constants to
   planner so you might get slower plans than the queries joining
   the system tables correnspond to the OID types.

regards,


At Mon, 2 Mar 2015 17:50:37 -0600, Jim Nasby jim.na...@bluetreble.com wrote 
in 54f4f74d.8000...@bluetreble.com
 On 3/2/15 3:56 PM, Andres Freund wrote:
  On 2015-03-02 16:42:35 -0500, Robert Haas wrote:
  On Tue, Feb 3, 2015 at 10:12 AM, Tom Lanet...@sss.pgh.pa.us  wrote:
   Two reasons this isn't terribly compelling are (1) it's creating a
   join in a place where the planner can't possibly see it and optimize
   it, and (2) you risk MVCC anomalies because the reg* output routines
   would not be using the same snapshot as the calling query.
   
   We already have problem (2) with the existing reg* functions so I'm
   not that excited about doubling down on the concept.
  
  I think I agree.  I mean, I agree that this notation is more
  convenient, but I don't really want to add a whole new slough of types
  --- these will certainly not be the only ones we want once we go down
  this path --- to the default install just for notational convenience.
  It's arguable, of course, but I guess I'm going to vote against this
  patch.
  That's a justifyable position. I don't think there are other catalogs
  referenced as pervasively in the catalog though.
 
  There's one additional point: Using reg* types in the catalog tables
  themselves can make them*much* easier to read. I personally do look at
  the catalogs a awful lot, and seing names instead of oids makes it
  much
  easier. And adding regtype/role would allow to cover nearly all types
  containing oids.
 
 +1. Constantly joining catalog tables together is a royal PITA, and
 regnamespace is the biggest missing part of this (I typically don't
 look at roles too much, but I can certainly see it's importance).
 
 If we had more user friendly views on the catalogs maybe this wouldn't
 be an issue... but that's a much larger project.
 
 BTW, I think the potential for MVCC issues should be mentioned in the
 docs (http://www.postgresql.org/docs/devel/static/datatype-oid.html).

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From f6c9754f190d4cfe29f776ad1fe5f1a012533924 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp
Date: Wed, 18 Feb 2015 14:38:32 +0900
Subject: [PATCH 1/2] Add regrole

Add new OID aliass type regrole. The new type has the scope of whole
the database cluster so it doesn't behave as the same as the existing
OID alias types which have database scope, concerning object
dependency. To get rid of confusion, inhibit constants of the new type
from appearing where dependencies are made involving it.

Documentation about this new type is added and some existing
descriptions are modified according to the restriction of the
type. Addition to it, put a note about possible MVCC violation and
optimization issues, which are general over the all reg* types.
---
 doc/src/sgml/datatype.sgml| 55 +---
 src/backend/bootstrap/bootstrap.c |  2 +
 src/backend/catalog/dependency.c  | 22 
 src/backend/utils/adt/regproc.c   | 98 +++
 src/backend/utils/adt/selfuncs.c  |  2 +
 src/backend/utils/cache/catcache.c|  1 +
 src/include/catalog/pg_cast.h |  7 +++
 src/include/catalog/pg_proc.h | 10 
 src/include/catalog/pg_type.h |  5 ++
 src/include/utils/builtins.h  |  5 ++
 src/test/regress/expected/regproc.out | 26 +-
 src/test/regress/sql/regproc.sql  |  7 +++
 12 files changed, 221 insertions(+), 19 deletions(-)

diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index edf636b..dab5430 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4319,8 +4319,9 @@ SET xmloption TO { DOCUMENT | CONTENT };
 an object identifier.  There are also several alias types for
 typeoid/: typeregproc/, typeregprocedure/,
 typeregoper/, typeregoperator/, typeregclass/,
-typeregtype/, typeregconfig/, and typeregdictionary/.
-xref linkend=datatype-oid-table shows an overview.
+typeregtype/, typeregrole/, typeregconfig/, and
+typeregdictionary/.  xref linkend=datatype-oid-table shows
+an overview.
/para
 
para
@@ -4429,6 +4430,13 @@ SELECT * FROM pg_attribute

Re: [HACKERS] How about to have relnamespace and relrole?

[Robert Haas]

On Tue, Feb 3, 2015 at 10:12 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Two reasons this isn't terribly compelling are (1) it's creating a
 join in a place where the planner can't possibly see it and optimize
 it, and (2) you risk MVCC anomalies because the reg* output routines
 would not be using the same snapshot as the calling query.

 We already have problem (2) with the existing reg* functions so I'm
 not that excited about doubling down on the concept.

I think I agree.  I mean, I agree that this notation is more
convenient, but I don't really want to add a whole new slough of types
--- these will certainly not be the only ones we want once we go down
this path --- to the default install just for notational convenience.
It's arguable, of course, but I guess I'm going to vote against this
patch.

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

Re: [HACKERS] How about to have relnamespace and relrole?

[Andres Freund]

On 2015-03-02 16:42:35 -0500, Robert Haas wrote:
 On Tue, Feb 3, 2015 at 10:12 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Two reasons this isn't terribly compelling are (1) it's creating a
  join in a place where the planner can't possibly see it and optimize
  it, and (2) you risk MVCC anomalies because the reg* output routines
  would not be using the same snapshot as the calling query.
 
  We already have problem (2) with the existing reg* functions so I'm
  not that excited about doubling down on the concept.
 
 I think I agree.  I mean, I agree that this notation is more
 convenient, but I don't really want to add a whole new slough of types
 --- these will certainly not be the only ones we want once we go down
 this path --- to the default install just for notational convenience.
 It's arguable, of course, but I guess I'm going to vote against this
 patch.

That's a justifyable position. I don't think there are other catalogs
referenced as pervasively in the catalog though.

There's one additional point: Using reg* types in the catalog tables
themselves can make them *much* easier to read. I personally do look at
the catalogs a awful lot, and seing names instead of oids makes it much
easier. And adding regtype/role would allow to cover nearly all types
containing oids.

Incidentally I've started working on a replacement for the bki DATA
stuff
(http://archives.postgresql.org/message-id/20150220234142.GH12653%40awork2.anarazel.de)
and of the things that makes the biggest difference in editing based on
my experience is not to have to list endless columns of oids that you
have to figure out by hand. Replacing e.g. prorettype/proallargtypes
oids by their names made it much, much easier to read. And my initial
implementation simply supports that based on the column type... So
adding regnamespace/regrole actually might help there.

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

Re: [HACKERS] How about to have relnamespace and relrole?

[Jim Nasby]

On 3/2/15 3:56 PM, Andres Freund wrote:

On 2015-03-02 16:42:35 -0500, Robert Haas wrote:

On Tue, Feb 3, 2015 at 10:12 AM, Tom Lanet...@sss.pgh.pa.us  wrote:

 Two reasons this isn't terribly compelling are (1) it's creating a
 join in a place where the planner can't possibly see it and optimize
 it, and (2) you risk MVCC anomalies because the reg* output routines
 would not be using the same snapshot as the calling query.
 
 We already have problem (2) with the existing reg* functions so I'm
 not that excited about doubling down on the concept.


I think I agree.  I mean, I agree that this notation is more
convenient, but I don't really want to add a whole new slough of types
--- these will certainly not be the only ones we want once we go down
this path --- to the default install just for notational convenience.
It's arguable, of course, but I guess I'm going to vote against this
patch.

That's a justifyable position. I don't think there are other catalogs
referenced as pervasively in the catalog though.

There's one additional point: Using reg* types in the catalog tables
themselves can make them*much*  easier to read. I personally do look at
the catalogs a awful lot, and seing names instead of oids makes it much
easier. And adding regtype/role would allow to cover nearly all types
containing oids.


+1. Constantly joining catalog tables together is a royal PITA, and 
regnamespace is the biggest missing part of this (I typically don't look 
at roles too much, but I can certainly see it's importance).


If we had more user friendly views on the catalogs maybe this wouldn't 
be an issue... but that's a much larger project.


BTW, I think the potential for MVCC issues should be mentioned in the 
docs (http://www.postgresql.org/docs/devel/static/datatype-oid.html).

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Jeevan Chalke]

 The attatched are the fourth version of this patch.

 0001-Add-regrole_v4.patch
 0002-Add-regnamespace_v4.patch


Seems like you have missed to attach both the patches.

-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, thank you for reviewing.

The attatched are the third version of this patch.

0001-Add-regrole_v3.patch
0002-Add-regnamespace_v3.patch

- Rearranged into regrole patch and regnamespace patch as seen
  above, each of them consists of changes for code, docs,
  regtests. regnamespace patch depends on the another patch.

- Removed the irrelevant change and corrected mistakes in
  comments.

- Renamed role_or_oid to role_name_or_oid in regrolein().

- Changed the example name for regrole in the docmentation to
  'smithee' as an impossible-in-reality-but-well-known name, from
  'john', the most common in US (according to Wikipedia).



At Tue, 24 Feb 2015 16:30:44 +0530, Jeevan Chalke 
jeevan.cha...@enterprisedb.com wrote in 
CAM2+6=v-fwqfkwf0pi3dacq-zry5wxcplvluqfkf2mf57_6...@mail.gmail.com
 I agree on Tom's concern on MVCC issue, but we already hit that when we
 introduced regclass and others. So I see no additional issue with these
 changes as such. About planner slowness, I guess updated documentation looks
 perfect for that.
 
 So I went ahead reviewing these patches.
 
 All patches are straight forward and since we have similar code already
 exists, I did not find any issue at code level. They are consistent with
 other
 functions.

One concern is about arbitrary allocation of OIDs for the new
objects - types, functions. They are isolated from other similar
reg* types, but there's no help for it without global renumbering
of static(system) OIDs.

 Patches applies with patch -p1. make, make install, make check has
 no issues. Testing was fine too.
 
 However here are few review comments on the patches attached:
 
 Review points on 0001-Add-regrole.patch
 ---
 1.
 +#include utils/acl.h
 
 Can you please add it at appropriate place as #include list is an ordered
 list

regrolein calls reg_role_oid in acl.c, which is declared in acl.h.

 2.
 +char   *role_or_oid = PG_GETARG_CSTRING(0);
 
 Can we have variable named as role_name_or_oid, like other similar
 functions?

I might somehow have thought it a bit long. Fixed.

 3.
 +/*
 + * Normal case: parse the name into components and see if it matches
 any
 + * pg_role entries in the current search path.
 + */
 
 I believe, roles are never searched per search path. Thus need to update
 comments here.

Ouch. I forgot to edit it properly. I edit it. The correct
catalog name is pg_authid.

+   /* Normal case: see if the name matches any pg_authid entry. */

I also edited comments for regnamespacein.


 Review points on 0002-Add-regnamespace.patch
 ---
 1.
 + * regnamespacein- converts classname to class OID
 
 Typos. Should be nspname instead of classname and namespase OID instead of
 class OID

Thank you for pointing out. I fixed the same mistake in
regrolein, p_ts_dict was there instead of pg_authid..  The names
of oid kinds appear in multiple forms, that is, oprname and
opr_name. Although I don't understand the reason, I followed the
convention.

 Review points on 0003-Check-newpatch
 ---
 1.
 @@ -1860,6 +1860,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
  OidattrdefOid;
  ObjectAddress colobject,
  defobject;
 +Oidexprtype;
 
 This seems unrelated. Please remove.

It's a trace of the previous code to ruling out the new oid
types. Removed.

 Apart from this, it will be good if you have ONLY two patches,
 (1) For regrole and (2) For regnamespace specific
 With all related changes into one patch. I mean, all role related changes
 i.e.
 0001 + 0003 role related changes + 0004 role related changes and docs update
 AND
 0002 + 0003 nsp related changes + 0004 nsp related changes

I prudently separated it since I wasn't confident on the
pertinence of ruling out. I rearranged them as your advise
including docs.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 4d56a68e2bf2b7ee0da0447ad9f82f0b46277133 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp
Date: Wed, 18 Feb 2015 14:38:32 +0900
Subject: [PATCH 1/2] Add regrole

Add new OID aliass type regrole. The new type has the scope of whole
the database cluster so it doesn't behave as the same as the existing
OID alias types which have database scope, concerning object
dependency. To get rid of confusion, inhibit constants of the new type
from appearing where dependencies are made involving it.

Documentation about this new type is added and some existing
descriptions are modified according to the restriction of the
type. Addition to it, put a note about possible MVCC violation and
optimization issues, which are general over the all reg* types.
---
 doc/src/sgml/datatype.sgml| 55 +---
 src/backend/bootstrap/bootstrap.c |  2 +
 src/backend/catalog/dependency.c  | 22 
 src/backend/utils/adt/regproc.c   | 98 +++
 src/backend/utils/adt/selfuncs.c  |  2 +
 src/backend/utils/cache/catcache.c 

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Sorry, I fixed a silly typo in documentation in the previous version.

- of theses types has a significance... 
+ of these types has a significance...

# My fingers frequently slip as above..

I incremented the version of this revised patch to get rid of
confusion.

===
Hello, thank you for reviewing.

The attatched are the fourth version of this patch.

0001-Add-regrole_v4.patch
0002-Add-regnamespace_v4.patch

- Rearranged into regrole patch and regnamespace patch as seen
  above, each of them consists of changes for code, docs,
  regtests. regnamespace patch depends on the another patch.

- Removed the irrelevant change and corrected mistakes in
  comments.

- Renamed role_or_oid to role_name_or_oid in regrolein().

- Changed the example name for regrole in the docmentation to
  'smithee' as an impossible-in-reality-but-well-known name, from
  'john', the most common in US (according to Wikipedia).



At Tue, 24 Feb 2015 16:30:44 +0530, Jeevan Chalke 
jeevan.cha...@enterprisedb.com wrote in 
CAM2+6=v-fwqfkwf0pi3dacq-zry5wxcplvluqfkf2mf57_6...@mail.gmail.com
 I agree on Tom's concern on MVCC issue, but we already hit that when we
 introduced regclass and others. So I see no additional issue with these
 changes as such. About planner slowness, I guess updated documentation looks
 perfect for that.
 
 So I went ahead reviewing these patches.
 
 All patches are straight forward and since we have similar code already
 exists, I did not find any issue at code level. They are consistent with
 other
 functions.

One concern is about arbitrary allocation of OIDs for the new
objects - types, functions. They are isolated from other similar
reg* types, but there's no help for it without global renumbering
of static(system) OIDs.

 Patches applies with patch -p1. make, make install, make check has
 no issues. Testing was fine too.
 
 However here are few review comments on the patches attached:
 
 Review points on 0001-Add-regrole.patch
 ---
 1.
 +#include utils/acl.h
 
 Can you please add it at appropriate place as #include list is an ordered
 list

regrolein calls reg_role_oid in acl.c, which is declared in acl.h.

 2.
 +char   *role_or_oid = PG_GETARG_CSTRING(0);
 
 Can we have variable named as role_name_or_oid, like other similar
 functions?

I might somehow have thought it a bit long. Fixed.

 3.
 +/*
 + * Normal case: parse the name into components and see if it matches
 any
 + * pg_role entries in the current search path.
 + */
 
 I believe, roles are never searched per search path. Thus need to update
 comments here.

Ouch. I forgot to edit it properly. I edit it. The correct
catalog name is pg_authid.

+   /* Normal case: see if the name matches any pg_authid entry. */

I also edited comments for regnamespacein.


 Review points on 0002-Add-regnamespace.patch
 ---
 1.
 + * regnamespacein- converts classname to class OID
 
 Typos. Should be nspname instead of classname and namespase OID instead of
 class OID

Thank you for pointing out. I fixed the same mistake in
regrolein, p_ts_dict was there instead of pg_authid..  The names
of oid kinds appear in multiple forms, that is, oprname and
opr_name. Although I don't understand the reason, I followed the
convention.

 Review points on 0003-Check-newpatch
 ---
 1.
 @@ -1860,6 +1860,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
  OidattrdefOid;
  ObjectAddress colobject,
  defobject;
 +Oidexprtype;
 
 This seems unrelated. Please remove.

It's a trace of the previous code to ruling out the new oid
types. Removed.

 Apart from this, it will be good if you have ONLY two patches,
 (1) For regrole and (2) For regnamespace specific
 With all related changes into one patch. I mean, all role related changes
 i.e.
 0001 + 0003 role related changes + 0004 role related changes and docs update
 AND
 0002 + 0003 nsp related changes + 0004 nsp related changes

I prudently separated it since I wasn't confident on the
pertinence of ruling out. I rearranged them as your advise
including docs.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

At Tue, 24 Feb 2015 14:11:28 +0900, Michael Paquier michael.paqu...@gmail.com 
wrote in cab7npqt6ox3mv++hgmbd3ydu_5-1y5hcddmstk+qdya_mjp...@mail.gmail.com
  https://commitfest.postgresql.org/4/
 
  Did you fix that manually for me?
 
 
 Looking at the log entry:
 2015-02-19 21:11:08 Michael Paquier (michael-kun) Changed authors to
 Kyotaro Horiguchi (horiguti)
 I do a pass from time to on the patches...

Ah, I found it. Thank you.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Jeevan Chalke]

Hi,

Personally, I was looking for something like this as I need to see rolename
and namespace name many times in my queries rather than it's oid.
But making a JOIN expression every-time was a pain. This certainly makes it
easier. And I see most DBAs are looking for it.

I agree on Tom's concern on MVCC issue, but we already hit that when we
introduced regclass and others. So I see no additional issue with these
changes as such. About planner slowness, I guess updated documentation looks
perfect for that.

So I went ahead reviewing these patches.

All patches are straight forward and since we have similar code already
exists, I did not find any issue at code level. They are consistent with
other
functions.

Patches applies with patch -p1. make, make install, make check has
no issues. Testing was fine too.

However here are few review comments on the patches attached:

Review points on 0001-Add-regrole.patch
---
1.
+#include utils/acl.h

Can you please add it at appropriate place as #include list is an ordered
list

2.
+char   *role_or_oid = PG_GETARG_CSTRING(0);

Can we have variable named as role_name_or_oid, like other similar
functions?

3.
+/*
+ * Normal case: parse the name into components and see if it matches
any
+ * pg_role entries in the current search path.
+ */

I believe, roles are never searched per search path. Thus need to update
comments here.


Review points on 0002-Add-regnamespace.patch
---
1.
+ * regnamespacein- converts classname to class OID

Typos. Should be nspname instead of classname and namespase OID instead of
class OID


Review points on 0003-Check-newpatch
---
1.
@@ -1860,6 +1860,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
 OidattrdefOid;
 ObjectAddress colobject,
 defobject;
+Oidexprtype;

This seems unrelated. Please remove.


Apart from this, it will be good if you have ONLY two patches,
(1) For regrole and (2) For regnamespace specific
With all related changes into one patch. I mean, all role related changes
i.e.
0001 + 0003 role related changes + 0004 role related changes and docs update
AND
0002 + 0003 nsp related changes + 0004 nsp related changes

Thanks

-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Re: [HACKERS] How about to have relnamespace and relrole?

[Jeevan Chalke]

Reviewed posted directly on mail thread instead of posting it on commitfest app.


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello,

At Thu, 19 Feb 2015 15:30:53 -0500, Peter Eisentraut pete...@gmx.net wrote in 
54e647fd.5000...@gmx.net
 On 2/18/15 3:44 AM, Kyotaro HORIGUCHI wrote:
  Hello, this is the patchset v2 of this feature.
  
  0001-Add-regrole.patch
  0002-Add-regnamespace.patch
  0003-Check-new-reg-types-are-not-used-as-default-values.patch
  0004-Documentation-for-new-OID-alias-types.patch
 
 Somehow, these patches ended up in the commit fest without an author
 listed.  That should probably not be possible.

Mmm.. I saw the author for this is listed here,

https://commitfest.postgresql.org/4/

Did you fix that manually for me?

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Michael Paquier]

On Tue, Feb 24, 2015 at 11:35 AM, Kyotaro HORIGUCHI 
horiguchi.kyot...@lab.ntt.co.jp wrote:

 Hello,

 At Thu, 19 Feb 2015 15:30:53 -0500, Peter Eisentraut pete...@gmx.net
 wrote in 54e647fd.5000...@gmx.net
  On 2/18/15 3:44 AM, Kyotaro HORIGUCHI wrote:
   Hello, this is the patchset v2 of this feature.
  
   0001-Add-regrole.patch
   0002-Add-regnamespace.patch
   0003-Check-new-reg-types-are-not-used-as-default-values.patch
   0004-Documentation-for-new-OID-alias-types.patch
 
  Somehow, these patches ended up in the commit fest without an author
  listed.  That should probably not be possible.

 Mmm.. I saw the author for this is listed here,

 https://commitfest.postgresql.org/4/

 Did you fix that manually for me?


Looking at the log entry:
2015-02-19 21:11:08 Michael Paquier (michael-kun) Changed authors to
Kyotaro Horiguchi (horiguti)
I do a pass from time to on the patches...
-- 
Michael

Re: [HACKERS] How about to have relnamespace and relrole?

[Peter Eisentraut]

On 2/18/15 3:44 AM, Kyotaro HORIGUCHI wrote:
 Sorry, I sent the previous mail without patches by accident. The
 patches are attached to this mail.
 
 
 Hello, this is the patchset v2 of this feature.
 
 0001-Add-regrole.patch
   Adding regrole as the name says.
 
 0002-Add-regnamespace.patch
   Adding regnamespace. This depends on 0001 patch.
 
 0003-Check-new-reg-types-are-not-used-as-default-values.patch
   Inhibiting the new OID aliss types from being used as constants
   in default values, which make dependencies on the other
   (existing) OID alias types.
 
 0004-Documentation-for-new-OID-alias-types.patch
   Documentation patch for this new types.

Somehow, these patches ended up in the commit fest without an author
listed.  That should probably not be possible.



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Sorry, I sent the previous mail without patches by accident. The
patches are attached to this mail.


Hello, this is the patchset v2 of this feature.

0001-Add-regrole.patch
  Adding regrole as the name says.

0002-Add-regnamespace.patch
  Adding regnamespace. This depends on 0001 patch.

0003-Check-new-reg-types-are-not-used-as-default-values.patch
  Inhibiting the new OID aliss types from being used as constants
  in default values, which make dependencies on the other
  (existing) OID alias types.

0004-Documentation-for-new-OID-alias-types.patch
  Documentation patch for this new types.

regards,

At Tue, 17 Feb 2015 20:19:11 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
horiguchi.kyot...@lab.ntt.co.jp wrote in 
20150217.201911.239516619.horiguchi.kyot...@lab.ntt.co.jp
 Hello, thank you for the comment.
 
 The current patch lacks change in documentation and dependency
 stuff. Current framework doesn't consider changing pg_shdepend
 from column default expressions so the possible measures are the
 followings, I think.
 
 1. Make pg_shdepend to have refobjsubid and add some deptypes of
pg_depend, specifically DEPENDENCY_NORMAL is needed. Then,
change the dependency stuff.
 
 2. Simplly inhibit specifying them in default
   expressions. Integer or OID can act as the replacement.
 
=# create table t1 (a int, b regrole default 'horiguti'::regrole);
ERROR:  Type 'regrole' cannot have a default value
 
 1 is quite overkill but hardly seems to be usable. So I will go
 on 2 and post additional patch.
 
 
 At Thu, 12 Feb 2015 17:12:24 -0600, Jim Nasby jim.na...@bluetreble.com 
 wrote in 54dd3358.9030...@bluetreble.com
  On 2/12/15 5:28 AM, Kyotaro HORIGUCHI wrote:
   Hello, I changed the subject.
 
 # Ouch! the subject remaines wrong..
 
   1. Expected frequency of use
  ...
   For that reason, although the current policy of deciding whether
   to have reg* types seems to be whether they have schema-qualified
   names, I think regrole and regnamespace are valuable to have.
  
  Perhaps schema qualification was the original intent, but I think at
  this point everyone uses them for convenience. Why would I want to
  type out JOIN pg_namespace n ON n.oid = blah.???namespace when I could
  simply do ???namespace::regnamespace?
 
 Yes, that is the most important point of this patch.
 
   2. Anticipaed un-optimizability
  
   Tom pointed that these reg* types prevents planner from
   optimizing the query, so we should refrain from having such
   machinary. It should have been a long-standing issue but reg*s
   sees to be rather faster than joining corresponding catalogs
   for moderate number of the result rows, but this raises another
   more annoying issue.
  
  
   3. Potentially breakage of MVCC
  
   The another issue Tom pointed is potentially breakage of MVCC by
   using these reg* types. Syscache is invalidated on updates so
   this doesn't seem to be a problem on READ COMMITTED mode, but
   breaks SERIALIZABLE mode. But IMHO it is not so serious problem
   as long as such inconsistency occurs only on system catalog and
   it is explicitly documented togethee with the un-optimizability
   issue. I'll add a patch for this later.
  
  The way I look at it, all these arguments went out the window when
  regclass was introduced.
  
  The reality is that reg* is *way* more convenient than manual
  joins. The arguments about optimization and MVCC presumably apply to
  all reg* casts, which means that ship has long since sailed.
 
 I agree basically, but I think some caveat is needed. I'll
 include that in the coming documentation patch.
 
  If we offered views that made it easier to get at this data then maybe
  this wouldn't matter so much. But DBA's don't want to join 3 catalogs
  together to try and answer a simple question.
 
 It has been annoying me these days.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 2a6f689afdc8197c2fe2fc235a4819ce1a5e9928 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp
Date: Wed, 18 Feb 2015 14:38:32 +0900
Subject: [PATCH 1/4] Add regrole

Add new regtype regrole. For the development reason, the system OIDs
used for the new procs, types, casts are making a gap from existing
OIDs.
---
 src/backend/bootstrap/bootstrap.c |   2 +
 src/backend/utils/adt/regproc.c   | 101 ++
 src/backend/utils/adt/selfuncs.c  |   2 +
 src/backend/utils/cache/catcache.c|   1 +
 src/include/catalog/pg_cast.h |   7 +++
 src/include/catalog/pg_proc.h |  10 
 src/include/catalog/pg_type.h |   5 ++
 src/include/utils/builtins.h  |   5 ++
 src/test/regress/expected/regproc.out |  26 -
 src/test/regress/sql/regproc.sql  |   7 +++
 10 files changed, 165 insertions(+), 1 deletion(-)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index bc66eac..11e40ee 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ 

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, this is the patchset v2 of this feature.

0001-Add-regrole.patch
  Adding regrole as the name says.

0002-Add-regnamespace.patch
  Adding regnamespace. This depends on 0001 patch.

0003-Check-new-reg-types-are-not-used-as-default-values.patch
  Inhibiting the new OID aliss types from being used as constants
  in default values, which make dependencies on the other
  (existing) OID alias types.

0004-Documentation-for-new-OID-alias-types.patch
  Documentation patch for this new types.

regards,

At Tue, 17 Feb 2015 20:19:11 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
horiguchi.kyot...@lab.ntt.co.jp wrote in 
20150217.201911.239516619.horiguchi.kyot...@lab.ntt.co.jp
 Hello, thank you for the comment.
 
 The current patch lacks change in documentation and dependency
 stuff. Current framework doesn't consider changing pg_shdepend
 from column default expressions so the possible measures are the
 followings, I think.
 
 1. Make pg_shdepend to have refobjsubid and add some deptypes of
pg_depend, specifically DEPENDENCY_NORMAL is needed. Then,
change the dependency stuff.
 
 2. Simplly inhibit specifying them in default
   expressions. Integer or OID can act as the replacement.
 
=# create table t1 (a int, b regrole default 'horiguti'::regrole);
ERROR:  Type 'regrole' cannot have a default value
 
 1 is quite overkill but hardly seems to be usable. So I will go
 on 2 and post additional patch.
 
 
 At Thu, 12 Feb 2015 17:12:24 -0600, Jim Nasby jim.na...@bluetreble.com 
 wrote in 54dd3358.9030...@bluetreble.com
  On 2/12/15 5:28 AM, Kyotaro HORIGUCHI wrote:
   Hello, I changed the subject.
 
 # Ouch! the subject remaines wrong..
 
   1. Expected frequency of use
  ...
   For that reason, although the current policy of deciding whether
   to have reg* types seems to be whether they have schema-qualified
   names, I think regrole and regnamespace are valuable to have.
  
  Perhaps schema qualification was the original intent, but I think at
  this point everyone uses them for convenience. Why would I want to
  type out JOIN pg_namespace n ON n.oid = blah.???namespace when I could
  simply do ???namespace::regnamespace?
 
 Yes, that is the most important point of this patch.
 
   2. Anticipaed un-optimizability
  
   Tom pointed that these reg* types prevents planner from
   optimizing the query, so we should refrain from having such
   machinary. It should have been a long-standing issue but reg*s
   sees to be rather faster than joining corresponding catalogs
   for moderate number of the result rows, but this raises another
   more annoying issue.
  
  
   3. Potentially breakage of MVCC
  
   The another issue Tom pointed is potentially breakage of MVCC by
   using these reg* types. Syscache is invalidated on updates so
   this doesn't seem to be a problem on READ COMMITTED mode, but
   breaks SERIALIZABLE mode. But IMHO it is not so serious problem
   as long as such inconsistency occurs only on system catalog and
   it is explicitly documented togethee with the un-optimizability
   issue. I'll add a patch for this later.
  
  The way I look at it, all these arguments went out the window when
  regclass was introduced.
  
  The reality is that reg* is *way* more convenient than manual
  joins. The arguments about optimization and MVCC presumably apply to
  all reg* casts, which means that ship has long since sailed.
 
 I agree basically, but I think some caveat is needed. I'll
 include that in the coming documentation patch.
 
  If we offered views that made it easier to get at this data then maybe
  this wouldn't matter so much. But DBA's don't want to join 3 catalogs
  together to try and answer a simple question.
 
 It has been annoying me these days.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, thank you for the comment.

The current patch lacks change in documentation and dependency
stuff. Current framework doesn't consider changing pg_shdepend
from column default expressions so the possible measures are the
followings, I think.

1. Make pg_shdepend to have refobjsubid and add some deptypes of
   pg_depend, specifically DEPENDENCY_NORMAL is needed. Then,
   change the dependency stuff.

2. Simplly inhibit specifying them in default
  expressions. Integer or OID can act as the replacement.

   =# create table t1 (a int, b regrole default 'horiguti'::regrole);
   ERROR:  Type 'regrole' cannot have a default value

1 is quite overkill but hardly seems to be usable. So I will go
on 2 and post additional patch.


At Thu, 12 Feb 2015 17:12:24 -0600, Jim Nasby jim.na...@bluetreble.com wrote 
in 54dd3358.9030...@bluetreble.com
 On 2/12/15 5:28 AM, Kyotaro HORIGUCHI wrote:
  Hello, I changed the subject.

# Ouch! the subject remaines wrong..

  1. Expected frequency of use
 ...
  For that reason, although the current policy of deciding whether
  to have reg* types seems to be whether they have schema-qualified
  names, I think regrole and regnamespace are valuable to have.
 
 Perhaps schema qualification was the original intent, but I think at
 this point everyone uses them for convenience. Why would I want to
 type out JOIN pg_namespace n ON n.oid = blah.???namespace when I could
 simply do ???namespace::regnamespace?

Yes, that is the most important point of this patch.

  2. Anticipaed un-optimizability
 
  Tom pointed that these reg* types prevents planner from
  optimizing the query, so we should refrain from having such
  machinary. It should have been a long-standing issue but reg*s
  sees to be rather faster than joining corresponding catalogs
  for moderate number of the result rows, but this raises another
  more annoying issue.
 
 
  3. Potentially breakage of MVCC
 
  The another issue Tom pointed is potentially breakage of MVCC by
  using these reg* types. Syscache is invalidated on updates so
  this doesn't seem to be a problem on READ COMMITTED mode, but
  breaks SERIALIZABLE mode. But IMHO it is not so serious problem
  as long as such inconsistency occurs only on system catalog and
  it is explicitly documented togethee with the un-optimizability
  issue. I'll add a patch for this later.
 
 The way I look at it, all these arguments went out the window when
 regclass was introduced.
 
 The reality is that reg* is *way* more convenient than manual
 joins. The arguments about optimization and MVCC presumably apply to
 all reg* casts, which means that ship has long since sailed.

I agree basically, but I think some caveat is needed. I'll
include that in the coming documentation patch.

 If we offered views that made it easier to get at this data then maybe
 this wouldn't matter so much. But DBA's don't want to join 3 catalogs
 together to try and answer a simple question.

It has been annoying me these days.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Hello, I changed the subject.

This mail is to address the point at hand, preparing for
registering this commitfest.

15 17:29:14 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote in
20150204.172914.52110711.horiguchi.kyot...@lab.ntt.co.jp
 Tue, 03 Feb 2015 10:12:12 -0500, Tom Lane t...@sss.pgh.pa.us wrote in 
 2540.1422976...@sss.pgh.pa.us
  I'm not really excited about that.  That line of thought would imply
  that we should have reg* types for every system catalog, which is
  surely overkill.

 Mmm. I suppose for every OID usage, not every system catalog.
 but I agree as the whole. There's no agreeable-by-all
 boundary. Perhaps it depends on how often the average DBA looks
 onto catalogs which have oids pointing another catalog which they
 want to see in human-readable form, without joins if possible.

 I very roughly counted how often the oids of catalogs referred
 from other catalogs.

1. Expected frequency of use

I counted how often oids of system catalogs are referred to by
other system catalog/views. Among them, pg_stat_* views, are
excluded since they have text representations for all oid
references.

The result is like this. The full result of the counting is in
the Excel file but it's not at hand for now.. I'll show it here
if anyone wants to see it.

pg_class.oid: 27
pg_authid.oid:33
pg_namespace.oid: 20
pg_proc.oid:  13
pg_type.oid:  15
pg_databse.oid:5
pg_operator.oid:   5
pg_am.oid: 4


Among these, authid and namespace are apparently referred to
frequently but don't have their reg* types but referred to from
more points than proc, type, operator, am..

# By the way, I don't understand where the reg comes from,
# REGistered? Or other origin?

For that reason, although the current policy of deciding whether
to have reg* types seems to be whether they have schema-qualified
names, I think regrole and regnamespace are valuable to have.


2. Anticipaed un-optimizability

Tom pointed that these reg* types prevents planner from
optimizing the query, so we should refrain from having such
machinary. It should have been a long-standing issue but reg*s
sees to be rather faster than joining corresponding catalogs
for moderate number of the result rows, but this raises another
more annoying issue.


3. Potentially breakage of MVCC

The another issue Tom pointed is potentially breakage of MVCC by
using these reg* types. Syscache is invalidated on updates so
this doesn't seem to be a problem on READ COMMITTED mode, but
breaks SERIALIZABLE mode. But IMHO it is not so serious problem
as long as such inconsistency occurs only on system catalog and
it is explicitly documented togethee with the un-optimizability
issue. I'll add a patch for this later.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Jim Nasby]

On 2/12/15 5:28 AM, Kyotaro HORIGUCHI wrote:

Hello, I changed the subject.

This mail is to address the point at hand, preparing for
registering this commitfest.

15 17:29:14 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote in
20150204.172914.52110711.horiguchi.kyot...@lab.ntt.co.jp

Tue, 03 Feb 2015 10:12:12 -0500, Tom Lane t...@sss.pgh.pa.us wrote in 
2540.1422976...@sss.pgh.pa.us

I'm not really excited about that.  That line of thought would imply
that we should have reg* types for every system catalog, which is
surely overkill.


Mmm. I suppose for every OID usage, not every system catalog.
but I agree as the whole. There's no agreeable-by-all
boundary. Perhaps it depends on how often the average DBA looks
onto catalogs which have oids pointing another catalog which they
want to see in human-readable form, without joins if possible.

I very roughly counted how often the oids of catalogs referred
from other catalogs.


1. Expected frequency of use

...

For that reason, although the current policy of deciding whether
to have reg* types seems to be whether they have schema-qualified
names, I think regrole and regnamespace are valuable to have.


Perhaps schema qualification was the original intent, but I think at 
this point everyone uses them for convenience. Why would I want to type 
out JOIN pg_namespace n ON n.oid = blah.???namespace when I could simply 
do ???namespace::regnamespace?



2. Anticipaed un-optimizability

Tom pointed that these reg* types prevents planner from
optimizing the query, so we should refrain from having such
machinary. It should have been a long-standing issue but reg*s
sees to be rather faster than joining corresponding catalogs
for moderate number of the result rows, but this raises another
more annoying issue.


3. Potentially breakage of MVCC

The another issue Tom pointed is potentially breakage of MVCC by
using these reg* types. Syscache is invalidated on updates so
this doesn't seem to be a problem on READ COMMITTED mode, but
breaks SERIALIZABLE mode. But IMHO it is not so serious problem
as long as such inconsistency occurs only on system catalog and
it is explicitly documented togethee with the un-optimizability
issue. I'll add a patch for this later.


The way I look at it, all these arguments went out the window when 
regclass was introduced.


The reality is that reg* is *way* more convenient than manual joins. The 
arguments about optimization and MVCC presumably apply to all reg* 
casts, which means that ship has long since sailed.


If we offered views that made it easier to get at this data then maybe 
this wouldn't matter so much. But DBA's don't want to join 3 catalogs 
together to try and answer a simple question.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] How about to have relnamespace and relrole?

[Kyotaro HORIGUCHI]

Thank you for your comment.

Sorry for the silly typo in the subject.

Tue, 03 Feb 2015 10:12:12 -0500, Tom Lane t...@sss.pgh.pa.us wrote in 
2540.1422976...@sss.pgh.pa.us
 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
  Most of OID types has reg* OID types. Theses are very convenient
  when looking into system catalog/views, but there aren't OID
  types for userid and namespace id.
 
  What do you think about having these new OID types?
 
 I'm not really excited about that.  That line of thought would imply
 that we should have reg* types for every system catalog, which is
 surely overkill.

Mmm. I suppose for every OID usage, not every system catalog.
but I agree as the whole. There's no agreeable-by-all
boundary. Perhaps it depends on how often the average DBA looks
onto catalogs which have oids pointing another catalog which they
want to see in human-readable form, without joins if possible.

I very roughly counted how often the oids of catalogs referred
from other catalogs.

As the first step, catalogs which have oid are,

select relname from pg_class where relnamespace = 'pg_catalog'::regnamespace 
and relhasoids = true;
...
(34 rows)

# Yes, regnamespace is very usable here:)

I say that 34 is too many for sure to have reg* types.

From the viewpoint of human-readable names, the more it enters
DBA's sight, the more significance it could be said to have.

Among the 34, the rough list of from which catalog they are
referred to is following.

pg_authid(role):  pg_class, pg_type, pg_database, and many, many.
pg_type: referred to from pg_operator, pg_proc, pg_cast, and many.
pg_namespace: referred to from many catalogs.
pg_class: pg_locks
pg_database: pg_locks
pg_ts_parser: pg_ts_config
pg_ts_template: pg_ts_template
pg_foreign_data_wrapper:  pg_foreign_server
pg_foreign_server: pg_user_mapping

This is not an comprehensive counting but I think I can
confidently say that regrole has significant meaning. (and
namespace also could be said so). I would make the comprehensive
one if you or others think it's needed. (altough it would be a
labor)

What do you think about the point of view?


 The existing reg* types were chosen to handle the cases where objects have
 schema-qualified (and/or type-overloaded) names so that correct lookup is
 not merely a matter of (select oid from pg_foo where name = 'bar') or
 vice versa.
 
 I think the policy is, or should be, that we create reg* types for exactly
 the cases where lookup isn't that simple.

Yes, I have noticed that. And I agree that it is one reasonable
boundary. But the point mentioned above is also a reasonable
boundary.

 In particular, both of the cases you mention are hardly new.  They
 existed when we made the current set of reg* types and were consciously
 not added then.
 
  SELECT relnamespace::regnamespace, relname, relowner::regrole
  FROM pg_class WHERE relnamespace = 'public'::regnamespace;
 
 Two reasons this isn't terribly compelling are (1) it's creating a
 join in a place where the planner can't possibly see it and optimize
 it,

Maybe un-optimiz-ability is not a matter as far as they are used
in one-liners for administrative operations like I mentioned
above. (On the contrary, syscache is far faster than normal table
lookup for most cases, but it doesn't justify to use this in OLTP
jobs even ignoring the MVCC issue.)

  and (2) you risk MVCC anomalies because the reg* output routines
 would not be using the same snapshot as the calling query.
 We already have problem (2) with the existing reg* functions so I'm
 not that excited about doubling down on the concept.

Surely. Then the page for the reg* in the doc (datatype-oid.html)
*shoud* mention such a caveat, but the limitation would be
tolerable for user(DBA)s as the same as before.

Once it is stated clearly, althgouh I won't intend to make it an
excuse, I think some (or one) new reg* type can be acceptable.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] How about to have relnamespace and relrole?

[Tom Lane]

Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
 Most of OID types has reg* OID types. Theses are very convenient
 when looking into system catalog/views, but there aren't OID
 types for userid and namespace id.

 What do you think about having these new OID types?

I'm not really excited about that.  That line of thought would imply
that we should have reg* types for every system catalog, which is
surely overkill.

The existing reg* types were chosen to handle the cases where objects have
schema-qualified (and/or type-overloaded) names so that correct lookup is
not merely a matter of (select oid from pg_foo where name = 'bar') or
vice versa.

I think the policy is, or should be, that we create reg* types for exactly
the cases where lookup isn't that simple.

In particular, both of the cases you mention are hardly new.  They
existed when we made the current set of reg* types and were consciously
not added then.

 SELECT relnamespace::regnamespace, relname, relowner::regrole
 FROM pg_class WHERE relnamespace = 'public'::regnamespace;

Two reasons this isn't terribly compelling are (1) it's creating a
join in a place where the planner can't possibly see it and optimize
it, and (2) you risk MVCC anomalies because the reg* output routines
would not be using the same snapshot as the calling query.

We already have problem (2) with the existing reg* functions so I'm
not that excited about doubling down on the concept.

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