On Tue, 28 Aug 2018 11:49:26 +0900 (Tokyo Standard Time)
Kyotaro HORIGUCHI <[email protected]> wrote:
> At Fri, 24 Aug 2018 20:44:12 +0900, Yugo Nagata <[email protected]> wrote
> in <[email protected]>
> > When working on other patch[1], I found there are almost same
> > functions, texttoQualifiedNameList() and stringToQualifiedNameList().
> > The only difference is the argument type, text or char*. I don't know
> > why these functions are defined seperately, but I think the former
> > function can be rewritten using the latter code as the attached patch.
> > Is this reasonable fix?
>
> The functions were introduced within a month for different
> objectives in March and April, 2002. I supppose that they are
> intentionally added as separate functions for simplicitly since
> the second one is apparent CnP'ed from the first one.
>
> commit 5f4745adf4fb2a1f933b25d7a2bc72b39fa9edfd
> commit 52200befd04b9fa71da83231c808764867079226
Thank you for your comments. I also looked at the commit history.
stringToQNL is added after textToQNL as a static function originally,
but this is now public and reffered from other modules, too. So, I
propose to mote this from regproc.c to verlena.c and rewrite textToQNL
to call stringToQNL. This is just for reducing redundancy of the code.
Attached is the updated patch.
> Returning to the patch, the downside of it is that textToQNL
> makes an extra and unused copy of the parameter string. (It's a
> kind of bug that it is forgetting to free rawname.)
I also fixed to free rawname in the texttoQNL.
Regards,
--
Yugo Nagata <[email protected]>
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index a0079821fe..ebc087ed3f 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -1680,43 +1680,6 @@ text_regclass(PG_FUNCTION_ARGS)
}
-/*
- * Given a C string, parse it into a qualified-name list.
- */
-List *
-stringToQualifiedNameList(const char *string)
-{
- char *rawname;
- List *result = NIL;
- List *namelist;
- ListCell *l;
-
- /* We need a modifiable copy of the input string. */
- rawname = pstrdup(string);
-
- if (!SplitIdentifierString(rawname, '.', &namelist))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_NAME),
- errmsg("invalid name syntax")));
-
- if (namelist == NIL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_NAME),
- errmsg("invalid name syntax")));
-
- foreach(l, namelist)
- {
- char *curname = (char *) lfirst(l);
-
- result = lappend(result, makeString(pstrdup(curname)));
- }
-
- pfree(rawname);
- list_free(namelist);
-
- return result;
-}
-
/*****************************************************************************
* SUPPORT ROUTINES *
*****************************************************************************/
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index a5e812d026..1846ddcae8 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -3219,26 +3219,19 @@ name_text(PG_FUNCTION_ARGS)
PG_RETURN_TEXT_P(cstring_to_text(NameStr(*s)));
}
-
/*
- * textToQualifiedNameList - convert a text object to list of names
- *
- * This implements the input parsing needed by nextval() and other
- * functions that take a text parameter representing a qualified name.
- * We split the name at dots, downcase if not double-quoted, and
- * truncate names if they're too long.
+ * Given a C string, parse it into a qualified-name list.
*/
List *
-textToQualifiedNameList(text *textval)
+stringToQualifiedNameList(const char *string)
{
char *rawname;
List *result = NIL;
List *namelist;
ListCell *l;
- /* Convert to C string (handles possible detoasting). */
- /* Note we rely on being able to modify rawname below. */
- rawname = text_to_cstring(textval);
+ /* We need a modifiable copy of the input string. */
+ rawname = pstrdup(string);
if (!SplitIdentifierString(rawname, '.', &namelist))
ereport(ERROR,
@@ -3263,6 +3256,31 @@ textToQualifiedNameList(text *textval)
return result;
}
+
+/*
+ * textToQualifiedNameList - convert a text object to list of names
+ *
+ * This implements the input parsing needed by nextval() and other
+ * functions that take a text parameter representing a qualified name.
+ * We split the name at dots, downcase if not double-quoted, and
+ * truncate names if they're too long.
+ */
+List *
+textToQualifiedNameList(text *textval)
+{
+ char *rawname;
+ List *result = NIL;
+
+ /* Convert to C string (handles possible detoasting). */
+ /* Note we rely on being able to modify rawname below. */
+ rawname = text_to_cstring(textval);
+
+ result = stringToQualifiedNameList(rawname);
+ pfree(rawname);
+
+ return result;
+}
+
/*
* SplitIdentifierString --- parse a string containing identifiers
*
diff --git a/src/include/utils/regproc.h b/src/include/utils/regproc.h
index 5b9a8cbee8..ace74ed49f 100644
--- a/src/include/utils/regproc.h
+++ b/src/include/utils/regproc.h
@@ -15,7 +15,6 @@
#include "nodes/pg_list.h"
-extern List *stringToQualifiedNameList(const char *string);
extern char *format_procedure(Oid procedure_oid);
extern char *format_procedure_qualified(Oid procedure_oid);
extern void format_procedure_parts(Oid operator_oid, List **objnames,
diff --git a/src/include/utils/varlena.h b/src/include/utils/varlena.h
index c776931bc4..a51c84beb3 100644
--- a/src/include/utils/varlena.h
+++ b/src/include/utils/varlena.h
@@ -26,6 +26,7 @@ extern int varstr_levenshtein_less_equal(const char *source, int slen,
const char *target, int tlen,
int ins_c, int del_c, int sub_c,
int max_d, bool trusted);
+extern List *stringToQualifiedNameList(const char *string);
extern List *textToQualifiedNameList(text *textval);
extern bool SplitIdentifierString(char *rawstring, char separator,
List **namelist);
