From 220f2a7d15adc6ca811a3cbe5fee79ce4904cd91 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 18 Oct 2005 20:38:58 +0000 Subject: Code review for regexp_replace patch. Improve documentation and comments, fix problems with replacement-string backslashes that aren't followed by one of the expected characters, avoid giving the impression that replace_text_regexp() is meant to be called directly as a SQL function, etc. --- src/backend/utils/adt/regexp.c | 48 +++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 29 deletions(-) (limited to 'src/backend/utils/adt/regexp.c') diff --git a/src/backend/utils/adt/regexp.c b/src/backend/utils/adt/regexp.c index a872762c3c2..ce04ce77e67 100644 --- a/src/backend/utils/adt/regexp.c +++ b/src/backend/utils/adt/regexp.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/regexp.c,v 1.59 2005/10/15 02:49:29 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/regexp.c,v 1.60 2005/10/18 20:38:58 tgl Exp $ * * Alistair Crooks added the code for the regex caching * agc - cached the regular expressions used - there's a good chance @@ -83,7 +83,7 @@ static cached_re_str re_array[MAX_CACHED_RES]; /* cached re's */ /* * RE_compile_and_cache - compile a RE, caching if possible * - * Returns regex_t + * Returns regex_t * * * text_re --- the pattern, expressed as an *untoasted* TEXT object * cflags --- compile options for the pattern @@ -91,7 +91,7 @@ static cached_re_str re_array[MAX_CACHED_RES]; /* cached re's */ * Pattern is given in the database encoding. We internally convert to * array of pg_wchar which is what Spencer's regex package wants. */ -static regex_t +static regex_t * RE_compile_and_cache(text *text_re, int cflags) { int text_re_len = VARSIZE(text_re); @@ -123,7 +123,7 @@ RE_compile_and_cache(text *text_re, int cflags) re_array[0] = re_temp; } - return re_array[0].cre_re; + return &re_array[0].cre_re; } } @@ -188,7 +188,7 @@ RE_compile_and_cache(text *text_re, int cflags) re_array[0] = re_temp; num_res++; - return re_array[0].cre_re; + return &re_array[0].cre_re; } /* @@ -212,7 +212,7 @@ RE_compile_and_execute(text *text_re, char *dat, int dat_len, pg_wchar *data; size_t data_len; int regexec_result; - regex_t re; + regex_t *re; char errMsg[100]; /* Convert data string to wide characters */ @@ -223,7 +223,7 @@ RE_compile_and_execute(text *text_re, char *dat, int dat_len, re = RE_compile_and_cache(text_re, cflags); /* Perform RE match and return result */ - regexec_result = pg_regexec(&re_array[0].cre_re, + regexec_result = pg_regexec(re, data, data_len, 0, @@ -237,8 +237,7 @@ RE_compile_and_execute(text *text_re, char *dat, int dat_len, if (regexec_result != REG_OKAY && regexec_result != REG_NOMATCH) { /* re failed??? */ - pg_regerror(regexec_result, &re_array[0].cre_re, - errMsg, sizeof(errMsg)); + pg_regerror(regexec_result, re, errMsg, sizeof(errMsg)); ereport(ERROR, (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION), errmsg("regular expression failed: %s", errMsg))); @@ -442,10 +441,10 @@ textregexsubstr(PG_FUNCTION_ARGS) /* * textregexreplace_noopt() - * Return a replace string matched by a regular expression. - * This function is a version that doesn't specify the option of - * textregexreplace. This is case sensitive, replace the first - * instance only. + * Return a string matched by a regular expression, with replacement. + * + * This version doesn't have an option argument: we default to case + * sensitive match, replace the first instance only. */ Datum textregexreplace_noopt(PG_FUNCTION_ARGS) @@ -453,20 +452,16 @@ textregexreplace_noopt(PG_FUNCTION_ARGS) text *s = PG_GETARG_TEXT_P(0); text *p = PG_GETARG_TEXT_P(1); text *r = PG_GETARG_TEXT_P(2); - regex_t re; + regex_t *re; re = RE_compile_and_cache(p, regex_flavor); - return DirectFunctionCall4(replace_text_regexp, - PointerGetDatum(s), - PointerGetDatum(&re), - PointerGetDatum(r), - BoolGetDatum(false)); + PG_RETURN_TEXT_P(replace_text_regexp(s, (void *) re, r, false)); } /* * textregexreplace() - * Return a replace string matched by a regular expression. + * Return a string matched by a regular expression, with replacement. */ Datum textregexreplace(PG_FUNCTION_ARGS) @@ -478,9 +473,9 @@ textregexreplace(PG_FUNCTION_ARGS) char *opt_p = VARDATA(opt); int opt_len = (VARSIZE(opt) - VARHDRSZ); int i; - bool global = false; + bool glob = false; bool ignorecase = false; - regex_t re; + regex_t *re; /* parse options */ for (i = 0; i < opt_len; i++) @@ -491,8 +486,7 @@ textregexreplace(PG_FUNCTION_ARGS) ignorecase = true; break; case 'g': - global = true; - + glob = true; break; default: ereport(ERROR, @@ -508,11 +502,7 @@ textregexreplace(PG_FUNCTION_ARGS) else re = RE_compile_and_cache(p, regex_flavor); - return DirectFunctionCall4(replace_text_regexp, - PointerGetDatum(s), - PointerGetDatum(&re), - PointerGetDatum(r), - BoolGetDatum(global)); + PG_RETURN_TEXT_P(replace_text_regexp(s, (void *) re, r, glob)); } /* similar_escape() -- cgit v1.2.3