Re: [HACKERS] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-04 Thread Jim Nasby 

On 1/3/16 10:37 PM, Tom Lane wrote:

It's not a release stopper, but I plan to fix it in HEAD whenever I have
an idle hour.


Since I'm sure there's much better things you can be working on I was 
going to just do this myself. Then it occurred to me that this should be 
a great item for a new hacker to handle, so I'm going to figure out what 
we're doing with those things now-a-days and put it there.


If no one picks it up I'll get it into the last commitfest.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Michael Paquier 
On Mon, Jan 4, 2016 at 11:16 AM, Tom Lane  wrote:
> Jim Nasby  writes:
>> regrole and regnamespace don't run their output through quote_ident().
>> That's contrary to all the other reg* operators.
>
>> Worse, they also don't *allow* quoted input. Not only is that different
>> from reg*, it's the *opposite*:
>
> I'm inclined to think that you're right, and that this is something that
> ought to be changed.  It's not quite too late ...

Well, I can send a patch with some tests if need be... Tom, I guess
that you are already on it?
-- 
Michael


-- 
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
I wrote:
> There's no real advantage to that anyway, compared with just doing
> stringToQualifiedNameList and then complaining if its list_length isn't 1.

Or just use SplitIdentifierString directly ...

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

[HACKERS] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

I didn't see this discussed on the thread...

regrole and regnamespace don't run their output through quote_ident(). 
That's contrary to all the other reg* operators.


Worse, they also don't *allow* quoted input. Not only is that different 
from reg*, it's the *opposite*:


select 'with spaces'::regclass;
ERROR:  invalid name syntax
LINE 1: select 'with spaces'::regclass;
select '"with spaces"'::regclass;
   regclass
---
 "with spaces"
(1 row)

I think this needs to be fixed before 9.5 releases. :(
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Jim Nasby  writes:
> regrole and regnamespace don't run their output through quote_ident(). 
> That's contrary to all the other reg* operators.
> Worse, they also don't *allow* quoted input. Not only is that different 
> from reg*, it's the *opposite*:

BTW, there's a concrete reason why this is broken, which is that although
regrole and regnamespace didn't bother with copying quoting/dequoting from
the other types, they *did* copy the special case logic about allowing
and emitting numeric OIDs.  This means that an output like "1234" from
regroleout is formally inconsistent: there is no way to tell if it's an
numeric OID or a role name that happens to be all digits.  (With proper
quoting logic, you could tell because an all-digits role name would have
gotten quoted.)  Conversely, if you create an all-digits role name, there
is no way to get regrolein to interpret it as such, whereas a dequoting
rule would have disambiguated.

I'm inclined to leave to_regrole and to_regnamespace alone, though, since
they have no numeric-OID path, and they will provide an "out" for anyone
who wants to handle nonquoted names.  (Though at least in HEAD we ought to
fix them to take type text as input.  Using cstring for ordinary functions
is just sloppy.)

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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Michael Paquier  writes:
> On Mon, Jan 4, 2016 at 11:16 AM, Tom Lane  wrote:
>> I'm inclined to think that you're right, and that this is something that
>> ought to be changed.  It's not quite too late ...

> Well, I can send a patch with some tests if need be... Tom, I guess
> that you are already on it?

If you're already working on it, feel free to continue.

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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Jim Nasby  writes:
> regrole and regnamespace don't run their output through quote_ident(). 
> That's contrary to all the other reg* operators.

> Worse, they also don't *allow* quoted input. Not only is that different 
> from reg*, it's the *opposite*:

I'm inclined to think that you're right, and that this is something that
ought to be changed.  It's not quite too late ...

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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Jim Nasby  writes:
> On 1/3/16 9:23 PM, Tom Lane wrote:
>> I'm inclined to leave to_regrole and to_regnamespace alone, though, since
>> they have no numeric-OID path, and they will provide an "out" for anyone
>> who wants to handle nonquoted names.  (Though at least in HEAD we ought to
>> fix them to take type text as input.  Using cstring for ordinary functions
>> is just sloppy.)

> None of the other to_reg* casts do that though, so this would be 
> inconsistent.

Good point.  Also, anybody who really does want it like that can still
fall back to the traditional sub-SELECT-from-the-catalog approach.

> Another potential problem for regnamespace is that it doesn't allow an 
> entry for the catalog. I'm not sure what the spec says about that, but 
> every other function allows dbname.schema.blah (dbname == catalog).

Meh, these types conform to no spec, so we can make them do what we
like.  I see about zero reason to allow a catalog name, it would mostly
just confuse people.

> I started working on a fix, but it's currently blowing up in bootstrap 
> and I haven't been able to figure out why yet:
> running bootstrap script ... FATAL:  improper qualified name (too many 
> dotted names): oid_ops

Recommendation: don't muck with DeconstructQualifiedName.  That is called
in too many places and we are *not* touching any such API twenty-four
hours before release wrap.

There's no real advantage to that anyway, compared with just doing
stringToQualifiedNameList and then complaining if its list_length isn't 1.

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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 9:23 PM, Tom Lane wrote:

Jim Nasby  writes:

regrole and regnamespace don't run their output through quote_ident().
That's contrary to all the other reg* operators.
Worse, they also don't *allow* quoted input. Not only is that different
from reg*, it's the *opposite*:


BTW, there's a concrete reason why this is broken, which is that although
regrole and regnamespace didn't bother with copying quoting/dequoting from
the other types, they *did* copy the special case logic about allowing
and emitting numeric OIDs.  This means that an output like "1234" from
regroleout is formally inconsistent: there is no way to tell if it's an
numeric OID or a role name that happens to be all digits.  (With proper
quoting logic, you could tell because an all-digits role name would have
gotten quoted.)  Conversely, if you create an all-digits role name, there
is no way to get regrolein to interpret it as such, whereas a dequoting
rule would have disambiguated.

I'm inclined to leave to_regrole and to_regnamespace alone, though, since
they have no numeric-OID path, and they will provide an "out" for anyone
who wants to handle nonquoted names.  (Though at least in HEAD we ought to
fix them to take type text as input.  Using cstring for ordinary functions
is just sloppy.)


None of the other to_reg* casts do that though, so this would be 
inconsistent.


Another potential problem for regnamespace is that it doesn't allow an 
entry for the catalog. I'm not sure what the spec says about that, but 
every other function allows dbname.schema.blah (dbname == catalog).


I started working on a fix, but it's currently blowing up in bootstrap 
and I haven't been able to figure out why yet:


running bootstrap script ... FATAL:  improper qualified name (too many 
dotted names): oid_ops

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 8b105fe..a555a1f 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -2613,6 +2613,9 @@ TSConfigIsVisible(Oid cfgid)
  * extract the schema name and object name.
  *
  * *nspname_p is set to NULL if there is no explicit schema name.
+ * 
+ * If *objname_p is not set then treat names as a schema name, possibly with a
+ * catalog name.
  */
 void
 DeconstructQualifiedName(List *names,
@@ -2623,19 +2626,21 @@ DeconstructQualifiedName(List *names,
char   *schemaname = NULL;
char   *objname = NULL;
 
-   switch (list_length(names))
+   switch (list_length(names) + objname_p ? 0 : 1)
{
case 1:
objname = strVal(linitial(names));
break;
case 2:
schemaname = strVal(linitial(names));
-   objname = strVal(lsecond(names));
+   if (objname_p)
+   objname = strVal(lsecond(names));
break;
case 3:
catalogname = strVal(linitial(names));
schemaname = strVal(lsecond(names));
-   objname = strVal(lthird(names));
+   if (objname_p)
+   objname = strVal(lthird(names));
 
/*
 * We check the catalog name and then ignore it.
@@ -2655,7 +2660,8 @@ DeconstructQualifiedName(List *names,
}
 
*nspname_p = schemaname;
-   *objname_p = objname;
+   if (objname_p)
+   *objname_p = objname;
 }
 
 /*
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index 59e5dc8..8c862cf 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -1569,6 +1569,7 @@ Datum
 regrolein(PG_FUNCTION_ARGS)
 {
char   *role_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
/* '-' ? */
@@ -1586,7 +1587,15 @@ regrolein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_authid entry. */
-   result = get_role_oid(role_name_or_oid, false);
+   names = stringToQualifiedNameList(role_name_or_oid);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1668,7 +1677,9 @@ Datum
 regnamespacein(PG_FUNCTION_ARGS)
 {
char   *nsp_name_or_oid = PG_GETARG_CSTRING(0);
+   char   *nsp_name;
Oid result =

Re: [HACKERS] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Jim Nasby  writes:
> What I went with. Now to figure out why this is happening...

>SELECT regnamespace('pg_catalog');
> ! ERROR:  schema "Y" does not exist
> ! LINE 1: SELECT regnamespace('pg_catalog');

Confusion between a C string and a string Value node, mayhap?

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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Michael Paquier 
On Mon, Jan 4, 2016 at 1:20 PM, Jim Nasby  wrote:
> On 1/3/16 9:43 PM, Tom Lane wrote:
>>
>> Jim Nasby  writes:
>>>
>>> On 1/3/16 9:23 PM, Tom Lane wrote:
>>> Another potential problem for regnamespace is that it doesn't allow an
>>> entry for the catalog. I'm not sure what the spec says about that, but
>>> every other function allows dbname.schema.blah (dbname == catalog).
>>
>>
>> Meh, these types conform to no spec, so we can make them do what we
>> like.  I see about zero reason to allow a catalog name, it would mostly
>> just confuse people.
>
>
> Ok. Not like CREATE SCHEMA allows that anyway.
>
>>> I started working on a fix, but it's currently blowing up in bootstrap
>>> and I haven't been able to figure out why yet:
>>> running bootstrap script ... FATAL:  improper qualified name (too many
>>> dotted names): oid_ops
>>
>>
>> Recommendation: don't muck with DeconstructQualifiedName.  That is called
>> in too many places and we are *not* touching any such API twenty-four
>> hours before release wrap.
>
>
> Yeah, looks like that's what was blowing up.
>
>> There's no real advantage to that anyway, compared with just doing
>> stringToQualifiedNameList and then complaining if its list_length isn't 1.
>
>
> What I went with.

Thanks, this is more or less what I... just did..

+result = get_namespace_oid(nsp_name, false);
This is incorrect, you should use strVal(linitial(names)) instead.

+if (list_length(names) > 1)
+ereport(ERROR,
+(errcode(ERRCODE_SYNTAX_ERROR),
+errmsg("improper qualified name (too many dotted names): %s",
+   NameListToString(names;
I would just mark that as "Invalid syntax".

A couple of tests in regproc.sql would be a good addition as well.
-- 
Michael


-- 
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 10:20 PM, Jim Nasby wrote:

What I went with. Now to figure out why this is happening...


Nevermind, see my stupidity now. Should have full patch soon.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 11:05 PM, Michael Paquier wrote:

I'll take a look, but Michael, if you have time to review, an extra set
>of eyeballs wouldn't hurt.  There is no margin for error right now.

I'm just on it:)  Will update in a couple of minutes, I noticed some
stuff in Jim's patch.


BTW, in case you miss it... I was inconsistent with the list 
length_names checks... one is


if (list_length(names) > 1)

the other is

if (list_length(names) != 1)

(stringToQualifiedNameList() can't actually return a 0 length list and 
IIRC there was another place doing a > check.)

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 10:46 PM, Jim Nasby wrote:

Added. I'm gonna call this good for now. Note this is just against HEAD
since I don't have 9.5 setup yet. Presumably the patch should still
apply...


BTW, in case it's helpful... 
https://github.com/decibel/postgres/tree/regquote

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 11:24 PM, Michael Paquier wrote:

Sorry, didn't realize you were on it.

No worries. I know it's already late where you are.


And late where 9.5 is... ;)


I would use != 1 instead here, even if the function is strict.


Yeah, I effectively pulled the pattern from DeconstructQualifiedName, 
but != 1 is better.



You forgot to update the regression test output. And actually on

Doh.


second thoughts the tests you added do not actually bring much value
because what is tested is just the behavior of
stringToQualifiedNameList, and the other reg* are not tested that as


Seemed good to test what the original bug was, but sure.


well. However I think that we had better test the failures of
regnamespace and regrole when more than 1 element is parsed as this is
a new particular code path. Attached is an updated patch which passes
check-world in my environment.


Patch looks good to me. FWIW, RhodiumToad and macdice looked at my patch 
as well and didn't see any problems you didn't mention.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Jim Nasby  writes:
> On 1/3/16 9:23 PM, Tom Lane wrote:
>> (Though at least in HEAD we ought to
>> fix them to take type text as input.  Using cstring for ordinary functions
>> is just sloppy.)

> BTW, *all* the reg*in() functions do that...

Yeah, I noticed that.  They're all broken.  There are no other built-in
functions that take cstring except for datatype input functions and
encoding conversion functions, which are required to by their respective
meta-APIs, and which are not meant to be invoked directly from SQL.
These functions had no business being the first to think that cstring is
a full fledged type; which it is not.  Notably, that means this doesn't
work:

regression=# select to_regrole('foo'::text);
ERROR:  function to_regrole(text) does not exist

and most other cases where you'd computed an input value rather than
merely typed in a literal would fail likewise.

It's not a release stopper, but I plan to fix it in HEAD whenever I have
an idle hour.

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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 10:26 PM, Michael Paquier wrote:

Thanks, this is more or less what I... just did..


Sorry, didn't realize you were on it.


+result = get_namespace_oid(nsp_name, false);
This is incorrect, you should use strVal(linitial(names)) instead.


Yup. Dur.


+if (list_length(names) > 1)
+ereport(ERROR,
+(errcode(ERRCODE_SYNTAX_ERROR),
+errmsg("improper qualified name (too many dotted names): %s",
+   NameListToString(names;
I would just mark that as "Invalid syntax".


Just noticed this... I just copied the same syntax used elsewhere... 
whoever commits feel free to editorialize...



A couple of tests in regproc.sql would be a good addition as well.


Added. I'm gonna call this good for now. Note this is just against HEAD 
since I don't have 9.5 setup yet. Presumably the patch should still apply...

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index 59e5dc8..529d692 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -1569,6 +1569,7 @@ Datum
 regrolein(PG_FUNCTION_ARGS)
 {
char   *role_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
/* '-' ? */
@@ -1586,7 +1587,15 @@ regrolein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_authid entry. */
-   result = get_role_oid(role_name_or_oid, false);
+   names = stringToQualifiedNameList(role_name_or_oid);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1600,9 +1609,18 @@ Datum
 to_regrole(PG_FUNCTION_ARGS)
 {
char   *role_name = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
-   result = get_role_oid(role_name, true);
+   names = stringToQualifiedNameList(role_name);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), true);
 
if (OidIsValid(result))
PG_RETURN_OID(result);
@@ -1668,6 +1686,7 @@ Datum
 regnamespacein(PG_FUNCTION_ARGS)
 {
char   *nsp_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result = InvalidOid;
 
/* '-' ? */
@@ -1685,7 +1704,15 @@ regnamespacein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_namespace entry. */
-   result = get_namespace_oid(nsp_name_or_oid, false);
+   names = stringToQualifiedNameList(nsp_name_or_oid);
+
+   if (list_length(names) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_namespace_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1699,9 +1726,18 @@ Datum
 to_regnamespace(PG_FUNCTION_ARGS)
 {
char   *nsp_name = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
-   result = get_namespace_oid(nsp_name, true);
+   names = stringToQualifiedNameList(nsp_name);
+
+   if (list_length(names) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_namespace_oid(strVal(linitial(names)), true);
 
if (OidIsValid(result))
PG_RETURN_OID(result);
diff --git a/src/test/regress/sql/regproc.sql b/src/test/regress/sql/regproc.sql
index 8edaf15..cb427dc 100644
--- a/src/test/regress/sql/regproc.sql
+++ b/src/test/regress/sql/regproc.sql
@@ -14,7 +14,9 @@ SELECT regprocedure('abs(numeric)');
 SELECT regclass('pg_class');
 SELECT regtype('int4');
 SELECT regrole('regtestrole');
+SELECT regrole('"regtestrole"');
 SELECT regnamespace('pg_catalog');
+SELECT regnamespace('"pg_catalog"');
 
 SELECT to_regoper('||/');
 SELECT to_regoperator('+(int4,int4)');
@@ -23,7 +25,9 @@ SELECT to_regprocedure('abs(numeric)');
 SELECT to_regclass('pg_class');

Re: [HACKERS] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Michael Paquier  writes:
> Attached is an updated patch which passes
> check-world in my environment.

Pushed with trivial cosmetic changes to the code, and slightly more
extensive work on the regression test cases.

It strikes me that there might be an argument for returning NULL
rather than raising an error for to_regrole('foo.bar') and
to_regnamespace('foo.bar').  On the other hand, input-syntax issues
result in errors for the other to_reg* functions, so it might
be fine as-is.  In any case, relaxing the error rather than adding
one is much easier to justify as a future change, so I left it
as Jim/Michael had it for the moment.

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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Michael Paquier 
On Mon, Jan 4, 2016 at 3:09 PM, Tom Lane  wrote:
> Michael Paquier  writes:
>> Attached is an updated patch which passes
>> check-world in my environment.
>
> Pushed with trivial cosmetic changes to the code, and slightly more
> extensive work on the regression test cases.
>
> It strikes me that there might be an argument for returning NULL
> rather than raising an error for to_regrole('foo.bar') and
> to_regnamespace('foo.bar').  On the other hand, input-syntax issues
> result in errors for the other to_reg* functions, so it might
> be fine as-is.  In any case, relaxing the error rather than adding
> one is much easier to justify as a future change, so I left it
> as Jim/Michael had it for the moment.

Thanks!
-- 
Michael


-- 
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 9:43 PM, Tom Lane wrote:

Jim Nasby  writes:

On 1/3/16 9:23 PM, Tom Lane wrote:
Another potential problem for regnamespace is that it doesn't allow an
entry for the catalog. I'm not sure what the spec says about that, but
every other function allows dbname.schema.blah (dbname == catalog).


Meh, these types conform to no spec, so we can make them do what we
like.  I see about zero reason to allow a catalog name, it would mostly
just confuse people.


Ok. Not like CREATE SCHEMA allows that anyway.


I started working on a fix, but it's currently blowing up in bootstrap
and I haven't been able to figure out why yet:
running bootstrap script ... FATAL:  improper qualified name (too many
dotted names): oid_ops


Recommendation: don't muck with DeconstructQualifiedName.  That is called
in too many places and we are *not* touching any such API twenty-four
hours before release wrap.


Yeah, looks like that's what was blowing up.


There's no real advantage to that anyway, compared with just doing
stringToQualifiedNameList and then complaining if its list_length isn't 1.


What I went with. Now to figure out why this is happening...

  SELECT regnamespace('pg_catalog');
! ERROR:  schema "Y" does not exist
! LINE 1: SELECT regnamespace('pg_catalog');
!
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index 59e5dc8..339bfe7 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -1569,6 +1569,7 @@ Datum
 regrolein(PG_FUNCTION_ARGS)
 {
char   *role_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
/* '-' ? */
@@ -1586,7 +1587,15 @@ regrolein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_authid entry. */
-   result = get_role_oid(role_name_or_oid, false);
+   names = stringToQualifiedNameList(role_name_or_oid);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1668,7 +1677,9 @@ Datum
 regnamespacein(PG_FUNCTION_ARGS)
 {
char   *nsp_name_or_oid = PG_GETARG_CSTRING(0);
+   char   *nsp_name;
Oid result = InvalidOid;
+   List   *names;
 
/* '-' ? */
if (strcmp(nsp_name_or_oid, "-") == 0)
@@ -1685,7 +1696,15 @@ regnamespacein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_namespace entry. */
-   result = get_namespace_oid(nsp_name_or_oid, false);
+   names = stringToQualifiedNameList(nsp_name_or_oid);
+
+   if (list_length(names) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_namespace_oid(nsp_name, false);
 
PG_RETURN_OID(result);
 }

-- 
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Michael Paquier 
On Mon, Jan 4, 2016 at 2:01 PM, Tom Lane  wrote:
> Jim Nasby  writes:
>> On 1/3/16 10:26 PM, Michael Paquier wrote:
>>> Thanks, this is more or less what I... just did..
>
>> Sorry, didn't realize you were on it.
>
>>> A couple of tests in regproc.sql would be a good addition as well.
>
>> Added. I'm gonna call this good for now. Note this is just against HEAD
>> since I don't have 9.5 setup yet. Presumably the patch should still apply...
>
> I'll take a look, but Michael, if you have time to review, an extra set
> of eyeballs wouldn't hurt.  There is no margin for error right now.

I'm just on it :) Will update in a couple of minutes, I noticed some
stuff in Jim's patch.
-- 
Michael


-- 
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Michael Paquier 
On Mon, Jan 4, 2016 at 1:46 PM, Jim Nasby  wrote:
> On 1/3/16 10:26 PM, Michael Paquier wrote:
>>
>> Thanks, this is more or less what I... just did..
>
>
> Sorry, didn't realize you were on it.

No worries. I know it's already late where you are.

>> A couple of tests in regproc.sql would be a good addition as well.
>
> Added. I'm gonna call this good for now. Note this is just against HEAD
> since I don't have 9.5 setup yet. Presumably the patch should still apply...

+if (list_length(names) > 1)
+ereport(ERROR,
+(errcode(ERRCODE_SYNTAX_ERROR),
+errmsg("improper qualified name (too many dotted names): %s",
+   NameListToString(names;
I would use != 1 instead here, even if the function is strict.

You forgot to update the regression test output. And actually on
second thoughts the tests you added do not actually bring much value
because what is tested is just the behavior of
stringToQualifiedNameList, and the other reg* are not tested that as
well. However I think that we had better test the failures of
regnamespace and regrole when more than 1 element is parsed as this is
a new particular code path. Attached is an updated patch which passes
check-world in my environment.
-- 
Michael


20160104_regproc_quotes.patch
Description: binary/octet-stream

-- 
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Jim Nasby 

On 1/3/16 9:23 PM, Tom Lane wrote:

(Though at least in HEAD we ought to
fix them to take type text as input.  Using cstring for ordinary functions
is just sloppy.)


BTW, *all* the reg*in() functions do that...
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
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] 9.5 BLOCKER: regrole and regnamespace and quotes

2016-01-03 Thread Tom Lane 
Jim Nasby  writes:
> On 1/3/16 10:26 PM, Michael Paquier wrote:
>> Thanks, this is more or less what I... just did..

> Sorry, didn't realize you were on it.

>> A couple of tests in regproc.sql would be a good addition as well.

> Added. I'm gonna call this good for now. Note this is just against HEAD 
> since I don't have 9.5 setup yet. Presumably the patch should still apply...

I'll take a look, but Michael, if you have time to review, an extra set
of eyeballs wouldn't hurt.  There is no margin for error right now.

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