connectby with schema

Masaru Sugawara wrote:

CREATE SCHEMA ms;
CREATE TABLE ms.test (id int4, parent_id int4, t text);
INSERT INTO ms.test VALUES(11, null, 'aaa');
INSERT INTO ms.test VALUES(101, 11, 'bbb');
INSERT INTO ms.test VALUES(110, 11, 'ccc');
INSERT INTO ms.test VALUES(111, 110, 'ddd');
SELECT *
FROM connectby('ms.test', 'id', 'parent_id', '101', 0, '.')
as t(id int4, parent_id int4, level int, branch text);

ERROR: Relation "ms.test" does not exist

I've tracked this down to the fact that connectby does a quote_ident on the
provided relname, and in quote_ident, (quote_ident_required(t)) ends up being
true. The problem will occur even with a simple query:

test=# SELECT id, parent_id FROM ms.test WHERE parent_id = '101' AND id IS NOT
NULL;
id | parent_id
----+-----------
(0 rows)
test=# SELECT id, parent_id FROM "ms.test" WHERE parent_id = '101' AND id IS
NOT NULL;
ERROR: Relation "ms.test" does not exist

But this is not the behavior for unqualified table names:

test=# select * from foo;
f1
----
1
(1 row)
test=# select * from "foo";
f1
----
1
(1 row)

Is quote_ident_required incorrectly dealing with schemas?

Thanks,

Joe

Joe Conway wrote:

Is quote_ident_required incorrectly dealing with schemas?

Sorry to reply to myself, but another related question; shouldn't the
following produce "Ms"."Test"?

test=# select quote_ident('Ms.Test');
quote_ident
-------------
"Ms.Test"
(1 row)

Joe

On Fri, 22 Nov 2002, Joe Conway wrote:

Masaru Sugawara wrote:

CREATE SCHEMA ms;
CREATE TABLE ms.test (id int4, parent_id int4, t text);
INSERT INTO ms.test VALUES(11, null, 'aaa');
INSERT INTO ms.test VALUES(101, 11, 'bbb');
INSERT INTO ms.test VALUES(110, 11, 'ccc');
INSERT INTO ms.test VALUES(111, 110, 'ddd');
SELECT *
FROM connectby('ms.test', 'id', 'parent_id', '101', 0, '.')
as t(id int4, parent_id int4, level int, branch text);

ERROR: Relation "ms.test" does not exist

I've tracked this down to the fact that connectby does a quote_ident on the
provided relname, and in quote_ident, (quote_ident_required(t)) ends up being
true. The problem will occur even with a simple query:

test=# SELECT id, parent_id FROM ms.test WHERE parent_id = '101' AND id IS NOT
NULL;
id | parent_id
----+-----------
(0 rows)
test=# SELECT id, parent_id FROM "ms.test" WHERE parent_id = '101' AND id IS
NOT NULL;
ERROR: Relation "ms.test" does not exist

I think the query result here is correct behavior since in the second the
period shouldn't be a separator for schema and table but instead be part
of the identifier.

Dropping some bits that probably aren't important and merging some states

<table name> -> <qualified name>
<qualified name> -> [<schema name> <period>] <identifier>
<identifer> -> <regular identifier> |
<delimited identifier>
<delimited identifier> -> <double quote> <delimited identifier body>
<double quote>

I'd think that they'd parse like:
ms.test -> <identifier> . <identifier>
"ms.test" -> <delimited identifier>

The first would match <schema name> <period> <identifier>, but the second
would not.

Joe Conway <mail@joeconway.com> writes:

Joe Conway wrote:

Is quote_ident_required incorrectly dealing with schemas?

Sorry to reply to myself, but another related question; shouldn't the
following produce "Ms"."Test"?

test=# select quote_ident('Ms.Test');
quote_ident
-------------
"Ms.Test"
(1 row)

No, it should not. If it did, it would fail to cope with tablenames
containing dots.

Since connectby takes a string parameter (correct?) for the table name,
my advice would be to have it not do quote_ident, but instead expect the
user to include double quotes in the string value if dealing with
mixed-case names. Compare the behavior of nextval() for example:

regression=# select nextval('Foo.Bar');
ERROR: Namespace "foo" does not exist
regression=# select nextval('"Foo"."Bar"');
ERROR: Namespace "Foo" does not exist
regression=# select nextval('"Foo.Bar"');
ERROR: Relation "Foo.Bar" does not exist

regards, tom lane

Tom Lane wrote:

Since connectby takes a string parameter (correct?) for the table name,
my advice would be to have it not do quote_ident, but instead expect the
user to include double quotes in the string value if dealing with
mixed-case names. Compare the behavior of nextval() for example:

regression=# select nextval('Foo.Bar');
ERROR: Namespace "foo" does not exist
regression=# select nextval('"Foo"."Bar"');
ERROR: Namespace "Foo" does not exist
regression=# select nextval('"Foo.Bar"');
ERROR: Relation "Foo.Bar" does not exist

OK. Attached patch removes calls within the function to quote_ident, requiring
the user to appropriately quote their own identifiers. I also tweaked the
regression test to deal with "value" becoming a reserved word.

If it's not too late, I'd like this to get into 7.3, but in any case, please
apply to HEAD.

Thanks,

Joe

p.s. There are similar issues in dblink, but they appear a bit more difficult
to address. I'll attempt to get them resloved this weekend, again in hopes to
get them applied before 7.3 is released.

Joe Conway <mail@joeconway.com> writes:

OK. Attached patch removes calls within the function to quote_ident, requiring
the user to appropriately quote their own identifiers. I also tweaked the
regression test to deal with "value" becoming a reserved word.

Applied. I also threw in a quick note in the README to mention the need
for quoting.

regards, tom lane

Joe Conway wrote:

p.s. There are similar issues in dblink, but they appear a bit more
difficult to address. I'll attempt to get them resloved this weekend,
again in hopes to get them applied before 7.3 is released.

Attached patch removes most (hopefully just the appropriate ones) calls in
dblink to quote_ident, requiring the user to quote their own identifiers. I
also added to the regression test a case for a quoted, schema qualified table
name.

If it's not too late, I'd like this to get into 7.3, but in any case,
please apply to HEAD.

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Attached patch removes most (hopefully just the appropriate ones) calls in
dblink to quote_ident, requiring the user to quote their own identifiers. I
also added to the regression test a case for a quoted, schema qualified table
name.

If it's not too late, I'd like this to get into 7.3, but in any case,
please apply to HEAD.

Applied --- in 7.3 also, since Marc hasn't rolled RC2 yet.

regards, tom lane

On Fri, 22 Nov 2002 15:21:48 -0800
Joe Conway <mail@joeconway.com> wrote:

OK. Attached patch removes calls within the function to quote_ident, requiring
the user to appropriately quote their own identifiers. I also tweaked the
regression test to deal with "value" becoming a reserved word.

If it's not too late, I'd like this to get into 7.3, but in any case, please
apply to HEAD.

Thank you for your quick job.

Regards,
Masaru Sugawara

-------------------------------------
CREATE SCHEMA ms;
CREATE TABLE ms.test (id int4, parent_id int4, t text);
INSERT INTO ms.test VALUES(11, null, 'aaa');
INSERT INTO ms.test VALUES(101, 11, 'bbb');
INSERT INTO ms.test VALUES(110, 11, 'ccc');
INSERT INTO ms.test VALUES(111, 110, 'ddd');
SELECT *
FROM connectby('ms.test', 'id', 'parent_id', '11', 0, '.')
as t(id int, parent_id int, level int, branch text);

id | parent_id | level | branch
-----+-----------+-------+------------
11 | | 0 | 11
101 | 11 | 1 | 11.101
110 | 11 | 1 | 11.110
111 | 110 | 2 | 11.110.111
(4 rows)

------------------------------------
CREATE SCHEMA "MS";
drop table "MS"."Test";
CREATE TABLE "MS"."Test" (id int4, parent_id int4, t text);
INSERT INTO "MS"."Test" VALUES(22, null, 'aaa');
INSERT INTO "MS"."Test" VALUES(202, 22, 'bbb');
INSERT INTO "MS"."Test" VALUES(220, 22, 'ccc');
INSERT INTO "MS"."Test" VALUES(222, 220, 'ddd');
SELECT *
FROM connectby('"MS"."Test"', 'id', 'parent_id', '22', 0, '.')
as t(id int, parent_id int, level int, branch text);

id | parent_id | level | branch
-----+-----------+-------+------------
22 | | 0 | 22
202 | 22 | 1 | 22.202
220 | 22 | 1 | 22.220
222 | 220 | 2 | 22.220.222
(4 rows)