From 620dddf88a46310abf741b6f13f388e0f809ea27 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Thu, 3 Oct 2002 17:11:12 +0000 Subject: [PATCH] > The previous patch fixed an infinite recursion bug in > contrib/tablefunc/tablefunc.c:connectby. But, other unmanageable error > seems to occur even if a table has commonplace tree data(see below). > > I would think the patch, ancestor check, should be > > if (strstr(branch_delim || branchstr->data || branch_delim, > branch_delim || current_key || branch_delim)) > > This is my image, not a real code. However, if branchstr->data includes > branch_delim, my image will not be perfect. Good point. Thank you Masaru for the suggested fix. Attached is a patch to fix the bug found by Masaru. His example now produces: regression=# SELECT * FROM connectby('connectby_tree', 'keyid', 'parent_keyid', '11', 0, '-') AS t(keyid int, parent_keyid int, level int, branch text); keyid | parent_keyid | level | branch -------+--------------+-------+---------- 11 | | 0 | 11 10 | 11 | 1 | 11-10 111 | 11 | 1 | 11-111 1 | 111 | 2 | 11-111-1 (4 rows) While making the patch I also realized that the "no show branch" form of the function was not going to work very well for recursion detection. Therefore there is now a default branch delimiter ('~') that is used internally, for that case, to enable recursion detection to work. If you need a different delimiter for your specific data, you will have to use the "show branch" form of the function. Joe Conway --- contrib/tablefunc/README.tablefunc | 9 +++++++-- contrib/tablefunc/tablefunc.c | 27 ++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/contrib/tablefunc/README.tablefunc b/contrib/tablefunc/README.tablefunc index fe437256d7..c1faf190b3 100644 --- a/contrib/tablefunc/README.tablefunc +++ b/contrib/tablefunc/README.tablefunc @@ -365,7 +365,9 @@ Inputs branch_delim - if optional branch value is desired, this string is used as the delimiter + If optional branch value is desired, this string is used as the delimiter. + When not provided, a default value of '~' is used for internal + recursion detection only, and no "branch" field is returned. Outputs @@ -388,7 +390,10 @@ Notes the level value output 3. If the branch field is not desired, omit both the branch_delim input - parameter *and* the branch field in the query column definition + parameter *and* the branch field in the query column definition. Note + that when branch_delim is not provided, a default value of '~' is used + for branch_delim for internal recursion detection, even though the branch + field is not returned. 4. If the branch field is desired, it must be the forth column in the query column definition, and it must be type TEXT diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c index f5ac0f8ee6..6ea1e72b68 100644 --- a/contrib/tablefunc/tablefunc.c +++ b/contrib/tablefunc/tablefunc.c @@ -652,6 +652,9 @@ connectby_text(PG_FUNCTION_ARGS) branch_delim = GET_STR(PG_GETARG_TEXT_P(5)); show_branch = true; } + else + /* default is no show, tilde for the delimiter */ + branch_delim = pstrdup("~"); per_query_ctx = rsinfo->econtext->ecxt_per_query_memory; oldcontext = MemoryContextSwitchTo(per_query_ctx); @@ -798,10 +801,16 @@ build_tuplestore_recursively(char *key_fld, char *current_branch; char **values; StringInfo branchstr = NULL; + StringInfo chk_branchstr = NULL; + StringInfo chk_current_key = NULL; /* start a new branch */ branchstr = makeStringInfo(); + /* need these to check for recursion */ + chk_branchstr = makeStringInfo(); + chk_current_key = makeStringInfo(); + if (show_branch) values = (char **) palloc(CONNECTBY_NCOLS * sizeof(char *)); else @@ -854,22 +863,24 @@ build_tuplestore_recursively(char *key_fld, { /* initialize branch for this pass */ appendStringInfo(branchstr, "%s", branch); + appendStringInfo(chk_branchstr, "%s%s%s", branch_delim, branch, branch_delim); /* get the next sql result tuple */ spi_tuple = tuptable->vals[i]; /* get the current key and parent */ current_key = SPI_getvalue(spi_tuple, spi_tupdesc, 1); + appendStringInfo(chk_current_key, "%s%s%s", branch_delim, current_key, branch_delim); current_key_parent = pstrdup(SPI_getvalue(spi_tuple, spi_tupdesc, 2)); - /* check to see if this key is also an ancestor */ - if (strstr(branchstr->data, current_key)) - elog(ERROR, "infinite recursion detected"); - /* get the current level */ sprintf(current_level, "%d", level); - /* extend the branch */ + /* check to see if this key is also an ancestor */ + if (strstr(chk_branchstr->data, chk_current_key->data)) + elog(ERROR, "infinite recursion detected"); + + /* OK, extend the branch */ appendStringInfo(branchstr, "%s%s", branch_delim, current_key); current_branch = branchstr->data; @@ -913,6 +924,12 @@ build_tuplestore_recursively(char *key_fld, /* reset branch for next pass */ xpfree(branchstr->data); initStringInfo(branchstr); + + xpfree(chk_branchstr->data); + initStringInfo(chk_branchstr); + + xpfree(chk_current_key->data); + initStringInfo(chk_current_key); } }