Add function for quote_qualified_identifier?
Hi hackers,
I note that we currently expose the usefulness of the quote_identifier
function to the user with quote_ident(text).
Is there any reason we shouldn't do the same with quote_qualified_identifier?
We could just add a quote_qualified_ident(text, text) ... it would
make forming dynamic queries more convenient in databases that use
multiple schemas.
Clearly a DBA could just create this function himself in SQL (and it
wouldn't be difficult), but is that a good reason not to have it in
our standard set of functions?
Would be happy to cook up a patch for this.
Cheers,
BJ
This has been saved for the 8.4 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
---------------------------------------------------------------------------
Brendan Jurd wrote:
Hi hackers,
I note that we currently expose the usefulness of the quote_identifier
function to the user with quote_ident(text).Is there any reason we shouldn't do the same with quote_qualified_identifier?
We could just add a quote_qualified_ident(text, text) ... it would
make forming dynamic queries more convenient in databases that use
multiple schemas.Clearly a DBA could just create this function himself in SQL (and it
wouldn't be difficult), but is that a good reason not to have it in
our standard set of functions?Would be happy to cook up a patch for this.
Cheers,
BJ---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
I had some spare cycles so I went ahead and patched this.
Patch includes documentation and new regression tests. While I was in
there I also added regression tests for quote_ident(), which appeared
to be absent.
quote_literal doesn't seem to have any regression tests either, but I
decided to leave that for another patch.
With thanks to Neil Conway for his assistance on IRC.
Cheers
BJ
Show quoted text
On 9/15/07, Bruce Momjian <bruce@momjian.us> wrote:
This has been saved for the 8.4 release:
Brendan Jurd wrote:Hi hackers,
I note that we currently expose the usefulness of the quote_identifier
function to the user with quote_ident(text).Is there any reason we shouldn't do the same with quote_qualified_identifier?
We could just add a quote_qualified_ident(text, text) ... it would
make forming dynamic queries more convenient in databases that use
multiple schemas.Clearly a DBA could just create this function himself in SQL (and it
wouldn't be difficult), but is that a good reason not to have it in
our standard set of functions?Would be happy to cook up a patch for this.
Cheers,
BJ---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com+ If your life is a hard drive, Christ can be your backup. +
Attachments:
quote_qualified.difftext/plain; name=quote_qualified.diffDownload
Index: doc/src/sgml/func.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.397
diff -c -r1.397 func.sgml
*** doc/src/sgml/func.sgml 19 Sep 2007 03:13:57 -0000 1.397
--- doc/src/sgml/func.sgml 22 Sep 2007 03:07:26 -0000
***************
*** 1276,1281 ****
--- 1276,1284 ----
<primary>quote_ident</primary>
</indexterm>
<indexterm>
+ <primary>quote_qualified_ident</primary>
+ </indexterm>
+ <indexterm>
<primary>quote_literal</primary>
</indexterm>
<indexterm>
***************
*** 1541,1546 ****
--- 1544,1563 ----
</row>
<row>
+ <entry><literal><function>quote_qualified_ident</function>(<parameter>schema</parameter> <type>text</type>, <parameter>identifier</parameter> <type>text</type>)</literal></entry>
+ <entry><type>text</type></entry>
+ <entry>
+ Return the given schema and identifier suitably quoted to be used as a
+ fully qualified identifier in an <acronym>SQL</acronym> statement
+ string. Quoting is performed as for <function>quote_ident</function>,
+ but <parameter>schema</parameter> and <parameter>identifier</parameter>
+ are quoted separately.
+ </entry>
+ <entry><literal>quote_ident('Some schema','A table')</literal></entry>
+ <entry><literal>"Some schema"."A table"</literal></entry>
+ </row>
+
+ <row>
<entry><literal><function>quote_literal</function>(<parameter>string</parameter>)</literal></entry>
<entry><type>text</type></entry>
<entry>
Index: src/backend/utils/adt/quote.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/quote.c,v
retrieving revision 1.22
diff -c -r1.22 quote.c
*** src/backend/utils/adt/quote.c 27 Feb 2007 23:48:08 -0000 1.22
--- src/backend/utils/adt/quote.c 22 Sep 2007 03:07:26 -0000
***************
*** 46,51 ****
--- 46,77 ----
}
/*
+ * quote_qualified_ident -
+ * returns a properly quoted, schema-qualified identifier
+ */
+ Datum
+ quote_qualified_ident(PG_FUNCTION_ARGS)
+ {
+ text *schema = PG_GETARG_TEXT_P(0);
+ text *ident = PG_GETARG_TEXT_P(1);
+ text *result;
+ const char *quoted;
+ char *schema_s;
+ char *ident_s;
+
+ schema_s = DatumGetCString(DirectFunctionCall1(textout,
+ PointerGetDatum(schema)));
+ ident_s = DatumGetCString(DirectFunctionCall1(textout,
+ PointerGetDatum(ident)));
+
+ quoted = quote_qualified_identifier(schema_s, ident_s);
+
+ result = DatumGetTextP(DirectFunctionCall1(textin,
+ CStringGetDatum(quoted)));
+ PG_RETURN_TEXT_P(result);
+ }
+
+ /*
* quote_literal -
* returns a properly quoted literal
*
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.471
diff -c -r1.471 pg_proc.h
*** src/include/catalog/pg_proc.h 20 Sep 2007 17:56:32 -0000 1.471
--- src/include/catalog/pg_proc.h 22 Sep 2007 03:07:34 -0000
***************
*** 2630,2639 ****
DATA(insert OID = 1768 ( to_char PGNSP PGUID 12 1 0 f f t f s 2 25 "1186 25" _null_ _null_ _null_ interval_to_char - _null_ _null_ ));
DESCR("format interval to text");
! DATA(insert OID = 1282 ( quote_ident PGNSP PGUID 12 1 0 f f t f i 1 25 "25" _null_ _null_ _null_ quote_ident - _null_ _null_ ));
DESCR("quote an identifier for usage in a querystring");
! DATA(insert OID = 1283 ( quote_literal PGNSP PGUID 12 1 0 f f t f i 1 25 "25" _null_ _null_ _null_ quote_literal - _null_ _null_ ));
DESCR("quote a literal for usage in a querystring");
DATA(insert OID = 1798 ( oidin PGNSP PGUID 12 1 0 f f t f i 1 26 "2275" _null_ _null_ _null_ oidin - _null_ _null_ ));
DESCR("I/O");
--- 2630,2641 ----
DATA(insert OID = 1768 ( to_char PGNSP PGUID 12 1 0 f f t f s 2 25 "1186 25" _null_ _null_ _null_ interval_to_char - _null_ _null_ ));
DESCR("format interval to text");
! DATA(insert OID = 1282 ( quote_ident PGNSP PGUID 12 1 0 f f t f i 1 25 "25" _null_ _null_ _null_ quote_ident - _null_ _null_ ));
DESCR("quote an identifier for usage in a querystring");
! DATA(insert OID = 1283 ( quote_literal PGNSP PGUID 12 1 0 f f t f i 1 25 "25" _null_ _null_ _null_ quote_literal - _null_ _null_ ));
DESCR("quote a literal for usage in a querystring");
+ DATA(insert OID = 1285 ( quote_qualified_ident PGNSP PGUID 12 1 0 f f t f i 2 25 "25 25" _null_ _null_ _null_ quote_qualified_ident - _null_ _null_ ));
+ DESCR("quote a schema-qualified identifier for usage in a querystring");
DATA(insert OID = 1798 ( oidin PGNSP PGUID 12 1 0 f f t f i 1 26 "2275" _null_ _null_ _null_ oidin - _null_ _null_ ));
DESCR("I/O");
Index: src/include/utils/builtins.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.303
diff -c -r1.303 builtins.h
*** src/include/utils/builtins.h 18 Sep 2007 17:41:17 -0000 1.303
--- src/include/utils/builtins.h 22 Sep 2007 03:07:36 -0000
***************
*** 915,920 ****
--- 915,921 ----
/* quote.c */
extern Datum quote_ident(PG_FUNCTION_ARGS);
+ extern Datum quote_qualified_ident(PG_FUNCTION_ARGS);
extern Datum quote_literal(PG_FUNCTION_ARGS);
/* guc.c */
Index: src/test/regress/expected/strings.out
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/strings.out,v
retrieving revision 1.33
diff -c -r1.33 strings.out
*** src/test/regress/expected/strings.out 11 Aug 2007 03:56:24 -0000 1.33
--- src/test/regress/expected/strings.out 22 Sep 2007 03:07:37 -0000
***************
*** 1240,1242 ****
--- 1240,1257 ----
a\bcd | a\b'cd | a\b''cd | abcd\ | ab\'cd | \\
(1 row)
+ --
+ -- Test quoting of identifiers.
+ --
+ select quote_ident('safe') as f1, quote_ident('with space') as f2, quote_ident('punctuation?!') as f3, quote_ident('Cased') as f4, quote_ident('from') as f5, quote_ident('42') as f6, quote_ident('"quoted"') as f7;
+ f1 | f2 | f3 | f4 | f5 | f6 | f7
+ ------+--------------+-----------------+---------+--------+------+--------------
+ safe | "with space" | "punctuation?!" | "Cased" | "from" | "42" | """quoted"""
+ (1 row)
+
+ select quote_qualified_ident('public','safe') as f1, quote_qualified_ident('with space','safe') as f2, quote_qualified_ident('safe','punctuation?!') as f3, quote_qualified_ident('Mixed','cASE') as f4, quote_qualified_ident('from','42') as f5, quote_qualified_ident('"quoted"','"values"') as f6;
+ f1 | f2 | f3 | f4 | f5 | f6
+ -------------+-------------------+----------------------+----------------+-------------+---------------------------
+ public.safe | "with space".safe | safe."punctuation?!" | "Mixed"."cASE" | "from"."42" | """quoted"""."""values"""
+ (1 row)
+
Index: src/test/regress/sql/strings.sql
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/strings.sql,v
retrieving revision 1.22
diff -c -r1.22 strings.sql
*** src/test/regress/sql/strings.sql 11 Aug 2007 03:56:24 -0000 1.22
--- src/test/regress/sql/strings.sql 22 Sep 2007 03:07:38 -0000
***************
*** 450,452 ****
--- 450,459 ----
set standard_conforming_strings = off;
select 'a\\bcd' as f1, 'a\\b\'cd' as f2, 'a\\b\'''cd' as f3, 'abcd\\' as f4, 'ab\\\'cd' as f5, '\\\\' as f6;
+
+ --
+ -- Test quoting of identifiers.
+ --
+ select quote_ident('safe') as f1, quote_ident('with space') as f2, quote_ident('punctuation?!') as f3, quote_ident('Cased') as f4, quote_ident('from') as f5, quote_ident('42') as f6, quote_ident('"quoted"') as f7;
+ select quote_qualified_ident('public','safe') as f1, quote_qualified_ident('with space','safe') as f2, quote_qualified_ident('safe','punctuation?!') as f3, quote_qualified_ident('Mixed','cASE') as f4, quote_qualified_ident('from','42') as f5, quote_qualified_ident('"quoted"','"values"') as f6;
+
"Brendan Jurd" <direvus@gmail.com> writes:
Patch includes documentation and new regression tests. While I was in
there I also added regression tests for quote_ident(), which appeared
to be absent.
This seems rather pointless, since it's equivalent to
quote_ident(schemaname) || '.' || quote_ident(relname).
regards, tom lane
On 9/23/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
This seems rather pointless, since it's equivalent to
quote_ident(schemaname) || '.' || quote_ident(relname).
Yes it is, and I brought that up in the OP:
I wrote:
Clearly a DBA could just create this function himself in SQL (and it
wouldn't be difficult), but is that a good reason not to have it in
our standard set of functions?
But since nobody arced up about it I thought I might as well move
things along and produce a patch.
Many of the functions provided by postgres are easy to write yourself.
That doesn't mean they shouldn't be there. After all, there is
*exactly* one way to do quote_qualified_ident. Why require every DBA
who needs this functionality to go through the motions?
I'll admit that it's a minor improvement, but that seems reasonable
given it has a miniscule cost.
Tom Lane wrote:
"Brendan Jurd" <direvus@gmail.com> writes:
Patch includes documentation and new regression tests. While I was in
there I also added regression tests for quote_ident(), which appeared
to be absent.This seems rather pointless, since it's equivalent to
quote_ident(schemaname) || '.' || quote_ident(relname).
Has anyone every asked for this functionality?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On 9/29/07, Bruce Momjian <bruce@momjian.us> wrote:
Has anyone every asked for this functionality?
I searched the list archives for previous mentions of the topic, and
didn't find any. So the answer to your question is "yes", but so far
it seems to be just me.
Cheers,
BJ
On 9/29/07, Bruce Momjian <bruce@momjian.us> wrote:
I think we need more than one person's request to add this function.
Well, I don't expect it would get requested. Most DBAs would likely
look for the function in the docs, see it's not there and then just
implement it themselves. Obviously it's not critical. But
anticipating those little requirements and providing for them is one
of the things that makes a piece of software a pleasure to use.
"Batteries included" and all that.
Anyway, I seem to be flogging a horse which, if not dead, is surely
mortally wounded. If quote_qualified_ident isn't desired, perhaps you
can still use the regression test I included for quote_ident in the
patch. The test is functional as a standalone item, and seems to fill
a gap.
Thanks for your time,
BJ
Import Notes
Reply to msg id not found: 200709291038.l8TAcFO29322@momjian.us
Brendan Jurd wrote:
On 9/29/07, Bruce Momjian <bruce@momjian.us> wrote:
I think we need more than one person's request to add this function.
Well, I don't expect it would get requested. Most DBAs would likely
look for the function in the docs, see it's not there and then just
implement it themselves. Obviously it's not critical. But
anticipating those little requirements and providing for them is one
of the things that makes a piece of software a pleasure to use.
"Batteries included" and all that.
I was just looking for someone else to say "Yea, I would like that too".
Anyway, I seem to be flogging a horse which, if not dead, is surely
mortally wounded. If quote_qualified_ident isn't desired, perhaps you
can still use the regression test I included for quote_ident in the
patch. The test is functional as a standalone item, and seems to fill
a gap.
Well, we don't test everything and I don't remember problems in quoting
in the past, at least not enough to add another test.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian escribi�:
Brendan Jurd wrote:
On 9/29/07, Bruce Momjian <bruce@momjian.us> wrote:
I think we need more than one person's request to add this function.
Well, I don't expect it would get requested. Most DBAs would likely
look for the function in the docs, see it's not there and then just
implement it themselves. Obviously it's not critical. But
anticipating those little requirements and providing for them is one
of the things that makes a piece of software a pleasure to use.
"Batteries included" and all that.I was just looking for someone else to say "Yea, I would like that too".
Probably pgsql-hackers is not the best place to ask.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Brendan Jurd wrote:
On 9/29/07, Bruce Momjian <bruce@momjian.us> wrote:
I think we need more than one person's request to add this function.
Well, I don't expect it would get requested. Most DBAs would likely
look for the function in the docs, see it's not there and then just
implement it themselves. Obviously it's not critical. But
anticipating those little requirements and providing for them is one
of the things that makes a piece of software a pleasure to use.
"Batteries included" and all that.Anyway, I seem to be flogging a horse which, if not dead, is surely
mortally wounded. If quote_qualified_ident isn't desired, perhaps you
can still use the regression test I included for quote_ident in the
patch. The test is functional as a standalone item, and seems to fill
a gap.
I think we did our best to find requests for this feature. If they ever
pop up in the future we can always recover this patch from the archives.
Sorry.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +