Thread: Re: [PATCHES] regexp_replace

Re: [PATCHES] regexp_replace

From
David Fetter
Date:
On Mon, Jun 06, 2005 at 12:02:18PM -0400, Bruce Momjian wrote:
>
> Patch removed because we already have this functionality.

We don't yet have this functionality, as the patch allows for using
second and later regex matches "()" in the replacement pattern.

The function is misnamed.  It should be called regex_replace_all() or
some such, as it violates the principle of least astonishment by
replacing all instances by default.  Every other regex replacement
defaults to "replace first," not "replace all."  Or maybe it should
take a bool for "replace all," or...?  Anyhow, it's worth a discussion
:)

Cheers,
D
>
> ---------------------------------------------------------------------------
>
> a_ogawa00 wrote:
> >
> > This patch provides a new function regexp_replace.
> > regexp_replace extends a replace function and enables text search
> > by the regular expression. And, a back reference can be used within
> > a replace string.
> > (This patch for PostgreSQL 7.4.3)
> >
> > Function: regexp_replace(str, pattern, replace_str)
> > Retuen Type: text
> > Description: Replace all matched string in str.
> >              pattern is regular expression pattern.
> >              replace_str is replace string that can use '\1' - '\9', and
> > '\&'.
> >              '\1' - '\9' is back reference to the n'th subexpression.
> >              '\&' is matched string.
> >
> > (example1)
> > select regexp_replace('ABC-DEF', '(\\w+)-(\\w+)', '\\2-\\1')
> > result: DEF-ABC
> >
> > (example2)
> > update tab1 set col1 = regexp_replace(col1, '[A-Z]', '');
> >
> > ---
> > Atsushi Ogawa
> > a_ogawa@hi-ho.ne.jp
> >
> > --- cut here ---
> >
> > *** ./src/backend/regex/regexec.c.orig    Tue Jul 20 08:45:39 2004
> > --- ./src/backend/regex/regexec.c    Tue Jul 20 08:49:36 2004
> > ***************
> > *** 110,115 ****
> > --- 110,116 ----
> >       regmatch_t *pmatch;
> >       rm_detail_t *details;
> >       chr           *start;            /* start of string */
> > +     chr           *search_start;    /* search start of string */
> >       chr           *stop;            /* just past end of string */
> >       int            err;            /* error code if any (0 none) */
> >       regoff_t   *mem;            /* memory vector for backtracking */
> > ***************
> > *** 168,173 ****
> > --- 169,175 ----
> >   pg_regexec(regex_t *re,
> >              const chr *string,
> >              size_t len,
> > +            size_t search_start,
> >              rm_detail_t *details,
> >              size_t nmatch,
> >              regmatch_t pmatch[],
> > ***************
> > *** 219,224 ****
> > --- 221,227 ----
> >           v->pmatch = pmatch;
> >       v->details = details;
> >       v->start = (chr *) string;
> > +     v->search_start = (chr *) string + search_start;
> >       v->stop = (chr *) string + len;
> >       v->err = 0;
> >       if (backref)
> > ***************
> > *** 288,294 ****
> >       NOERR();
> >       MDEBUG(("\nsearch at %ld\n", LOFF(v->start)));
> >       cold = NULL;
> > !     close = shortest(v, s, v->start, v->start, v->stop, &cold, (int *)
> > NULL);
> >       freedfa(s);
> >       NOERR();
> >       if (v->g->cflags & REG_EXPECT)
> > --- 291,298 ----
> >       NOERR();
> >       MDEBUG(("\nsearch at %ld\n", LOFF(v->start)));
> >       cold = NULL;
> > !     close = shortest(v, s, v->search_start, v->search_start, v->stop,
> > !                      &cold, (int *) NULL);
> >       freedfa(s);
> >       NOERR();
> >       if (v->g->cflags & REG_EXPECT)
> > ***************
> > *** 415,421 ****
> >
> >       assert(d != NULL && s != NULL);
> >       cold = NULL;
> > !     close = v->start;
> >       do
> >       {
> >           MDEBUG(("\ncsearch at %ld\n", LOFF(close)));
> > --- 419,425 ----
> >
> >       assert(d != NULL && s != NULL);
> >       cold = NULL;
> > !     close = v->search_start;
> >       do
> >       {
> >           MDEBUG(("\ncsearch at %ld\n", LOFF(close)));
> > *** ./src/backend/utils/adt/regexp.c.orig    Tue Jul 20 08:50:08 2004
> > --- ./src/backend/utils/adt/regexp.c    Tue Jul 20 09:00:05 2004
> > ***************
> > *** 80,116 ****
> >
> >
> >   /*
> > !  * RE_compile_and_execute - compile and execute a RE, caching if possible
> >    *
> > !  * Returns TRUE on match, FALSE on no match
> >    *
> > !  *    text_re --- the pattern, expressed as an *untoasted* TEXT object
> > !  *    dat --- the data to match against (need not be null-terminated)
> > !  *    dat_len --- the length of the data string
> > !  *    cflags --- compile options for the pattern
> > !  *    nmatch, pmatch    --- optional return area for match details
> >    *
> > !  * Both pattern and data are given in the database encoding.  We
> > internally
> > !  * convert to array of pg_wchar which is what Spencer's regex package
> > wants.
> >    */
> > ! static bool
> > ! RE_compile_and_execute(text *text_re, unsigned char *dat, int dat_len,
> > !                        int cflags, int nmatch, regmatch_t *pmatch)
> >   {
> >       int            text_re_len = VARSIZE(text_re);
> > -     pg_wchar   *data;
> > -     size_t        data_len;
> >       pg_wchar   *pattern;
> >       size_t        pattern_len;
> >       int            i;
> >       int            regcomp_result;
> > -     int            regexec_result;
> >       cached_re_str re_temp;
> >
> > -     /* Convert data string to wide characters */
> > -     data = (pg_wchar *) palloc((dat_len + 1) * sizeof(pg_wchar));
> > -     data_len = pg_mb2wchar_with_len(dat, data, dat_len);
> > -
> >       /*
> >        * Look for a match among previously compiled REs.    Since the data
> >        * structure is self-organizing with most-used entries at the front,
> > --- 80,105 ----
> >
> >
> >   /*
> > !  * RE_compile_and_cache - compile a RE, caching if possible
> >    *
> > !  * Returns regex_t
> >    *
> > !  *  text_re --- the pattern, expressed as an *untoasted* TEXT object
> > !  *  cflags --- compile options for the pattern
> >    *
> > !  * 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
> > ! RE_compile_and_cache(text *text_re, int cflags)
> >   {
> >       int            text_re_len = VARSIZE(text_re);
> >       pg_wchar   *pattern;
> >       size_t        pattern_len;
> >       int            i;
> >       int            regcomp_result;
> >       cached_re_str re_temp;
> >
> >       /*
> >        * Look for a match among previously compiled REs.    Since the data
> >        * structure is self-organizing with most-used entries at the front,
> > ***************
> > *** 132,149 ****
> >                   re_array[0] = re_temp;
> >               }
> >
> > !             /* Perform RE match and return result */
> > !             regexec_result = pg_regexec(&re_array[0].cre_re,
> > !                                         data,
> > !                                         data_len,
> > !                                         NULL,    /* no details */
> > !                                         nmatch,
> > !                                         pmatch,
> > !                                         0);
> > !
> > !             pfree(data);
> > !
> > !             return (regexec_result == 0);
> >           }
> >       }
> >
> > --- 121,127 ----
> >                   re_array[0] = re_temp;
> >               }
> >
> > !             return re_array[0].cre_re;
> >           }
> >       }
> >
> > ***************
> > *** 210,219 ****
> > --- 188,231 ----
> >       re_array[0] = re_temp;
> >       num_res++;
> >
> > +     return re_array[0].cre_re;
> > + }
> > +
> > + /*
> > +  * RE_compile_and_execute - compile and execute a RE, caching if possible
> > +  *
> > +  * Returns TRUE on match, FALSE on no match
> > +  *
> > +  *    text_re --- the pattern, expressed as an *untoasted* TEXT object
> > +  *    dat --- the data to match against (need not be null-terminated)
> > +  *    dat_len --- the length of the data string
> > +  *    cflags --- compile options for the pattern
> > +  *    nmatch, pmatch    --- optional return area for match details
> > +  *
> > +  * Both pattern and data are given in the database encoding.  We
> > internally
> > +  * convert to array of pg_wchar which is what Spencer's regex package
> > wants.
> > +  */
> > + static bool
> > + RE_compile_and_execute(text *text_re, unsigned char *dat, int dat_len,
> > +                        int cflags, int nmatch, regmatch_t *pmatch)
> > + {
> > +     pg_wchar   *data;
> > +     size_t        data_len;
> > +     int            regexec_result;
> > +     regex_t        re;
> > +
> > +     /* Convert data string to wide characters */
> > +     data = (pg_wchar *) palloc((dat_len + 1) * sizeof(pg_wchar));
> > +     data_len = pg_mb2wchar_with_len(dat, data, dat_len);
> > +
> > +     /* Compile RE */
> > +     re = RE_compile_and_cache(text_re, cflags);
> > +
> >       /* Perform RE match and return result */
> >       regexec_result = pg_regexec(&re_array[0].cre_re,
> >                                   data,
> >                                   data_len,
> > +                                 0,
> >                                   NULL,    /* no details */
> >                                   nmatch,
> >                                   pmatch,
> > ***************
> > *** 415,420 ****
> > --- 427,452 ----
> >       }
> >
> >       PG_RETURN_NULL();
> > + }
> > +
> > + /*
> > +  * textregexreplace()
> > +  *      Return a replace string matched by a regular expression.
> > +  */
> > + Datum
> > + textregexreplace(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;
> > +
> > +     re = RE_compile_and_cache(p, regex_flavor);
> > +
> > +     return (DirectFunctionCall3(replace_text_regexp,
> > +                                 PointerGetDatum(s),
> > +                                 PointerGetDatum(&re),
> > +                                 PointerGetDatum(r)));
> >   }
> >
> >   /* similar_escape()
> > *** ./src/backend/utils/adt/varlena.c.orig    Tue Jul 20 09:00:17 2004
> > --- ./src/backend/utils/adt/varlena.c    Tue Jul 20 10:23:32 2004
> > ***************
> > *** 28,33 ****
> > --- 28,34 ----
> >   #include "utils/builtins.h"
> >   #include "utils/lsyscache.h"
> >   #include "utils/pg_locale.h"
> > + #include "regex/regex.h"
> >
> >
> >   typedef struct varlena unknown;
> > ***************
> > *** 1971,1976 ****
> > --- 1972,2122 ----
> >       ret_text = PG_STR_GET_TEXT(str->data);
> >       pfree(str->data);
> >       pfree(str);
> > +
> > +     PG_RETURN_TEXT_P(ret_text);
> > + }
> > +
> > + /*
> > +  * have_escape_in_regexp_replace_str
> > +  * check replace string have escape char
> > +  */
> > + static bool
> > + have_escape_in_regexp_replace_str(const char *replace_str)
> > + {
> > +     return (strchr(replace_str, '\\') != NULL);
> > + }
> > +
> > + #define REGEXP_REPLACE_BACKREF_CNT        10
> > + /*
> > +  * appendStringInfoRegexpSubstr
> > +  * append string for regexp back references.
> > +  */
> > + static void
> > + appendStringInfoRegexpSubstr(StringInfo str, text *replace_text,
> > +     regmatch_t *pmatch, text *buf_text, int search_start)
> > + {
> > +     const char *pstart = PG_TEXT_GET_STR(replace_text);
> > +     const char *p = pstart;
> > +     const char *pnext;
> > +
> > +     text       *add_text;
> > +     int            so;
> > +     int            eo;
> > +
> > +     for(;;) {
> > +         pnext = strchr(p, '\\');
> > +         if(pnext == NULL) break;
> > +
> > +         add_text = text_substring(PointerGetDatum(replace_text),
> > +                                   p - pstart + 1, pnext - p, false);
> > +         appendStringInfoString(str, PG_TEXT_GET_STR(add_text));
> > +         pfree(add_text);
> > +
> > +         p = pnext + 1;
> > +         so = eo = -1;
> > +
> > +         if(*p >= '1' && *p <= '9') {
> > +             int        idx = *p - '0';
> > +             so = pmatch[idx].rm_so - search_start;
> > +             eo = pmatch[idx].rm_eo - search_start;
> > +             p++;
> > +         } else {
> > +             switch(*p) {
> > +             case '&':
> > +                 so = pmatch[0].rm_so - search_start;
> > +                 eo = pmatch[0].rm_eo - search_start;
> > +                 p++;
> > +                 break;
> > +             }
> > +         }
> > +
> > +         if(so != -1 && eo != -1) {
> > +             add_text = text_substring(PointerGetDatum(buf_text),
> > +                                       so + 1, (eo - so), false);
> > +             appendStringInfoString(str, PG_TEXT_GET_STR(add_text));
> > +             pfree(add_text);
> > +         }
> > +     }
> > +
> > +     add_text = text_substring(PointerGetDatum(replace_text),
> > +                               p - pstart + 1, -1, true);
> > +     appendStringInfoString(str, PG_TEXT_GET_STR(add_text));
> > +     pfree(add_text);
> > + }
> > +
> > + /*
> > +  * replace_text_regexp
> > +  * replace text using regexp
> > + */
> > + Datum
> > + replace_text_regexp(PG_FUNCTION_ARGS)
> > + {
> > +     text       *left_text;
> > +     text       *right_text;
> > +     text       *buf_text;
> > +     text       *ret_text;
> > +     text       *src_text = PG_GETARG_TEXT_P(0);
> > +     char       *src_text_str = PG_TEXT_GET_STR(src_text);
> > +     int            src_text_len = TEXTLEN(src_text);
> > +     regex_t       *re = (regex_t *)PG_GETARG_POINTER(1);
> > +     text       *replace_text = PG_GETARG_TEXT_P(2);
> > +     char       *replace_str = PG_TEXT_GET_STR(replace_text);
> > +     StringInfo    str = makeStringInfo();
> > +     int            regexec_result;
> > +     regmatch_t    pmatch[REGEXP_REPLACE_BACKREF_CNT];
> > +     pg_wchar   *data;
> > +     size_t        data_len;
> > +     int            search_start;
> > +     bool        have_escape;
> > +
> > +     buf_text = TEXTDUP(src_text);
> > +
> > +     /* Convert data string to wide characters */
> > +     data = (pg_wchar *) palloc((src_text_len + 1) * sizeof(pg_wchar));
> > +     data_len = pg_mb2wchar_with_len(src_text_str, data,
> > strlen(src_text_str));
> > +
> > +     have_escape = have_escape_in_regexp_replace_str(replace_str);
> > +
> > +     for(search_start = 0; search_start <= data_len;) {
> > +         regexec_result = pg_regexec(re,
> > +                                     data,
> > +                                     data_len,
> > +                                     search_start,
> > +                                     NULL,   /* no details */
> > +                                     REGEXP_REPLACE_BACKREF_CNT,
> > +                                     pmatch,
> > +                                     0);
> > +         if(regexec_result != 0) break;
> > +
> > +         left_text = text_substring(PointerGetDatum(buf_text),
> > +                                    1, pmatch[0].rm_so - search_start, false);
> > +         right_text = text_substring(PointerGetDatum(buf_text),
> > +                                    pmatch[0].rm_eo - search_start + 1,
> > +                                    -1, true);
> > +
> > +         appendStringInfoString(str, PG_TEXT_GET_STR(left_text));
> > +         if(have_escape) {
> > +             appendStringInfoRegexpSubstr(str, replace_text, pmatch,
> > +                                          buf_text, search_start);
> > +         } else {
> > +             appendStringInfoString(str, replace_str);
> > +         }
> > +
> > +         pfree(buf_text);
> > +         pfree(left_text);
> > +         buf_text = right_text;
> > +
> > +         search_start = pmatch[0].rm_eo;
> > +         if(pmatch[0].rm_so == pmatch[0].rm_eo) search_start++;
> > +     }
> > +
> > +     appendStringInfoString(str, PG_TEXT_GET_STR(buf_text));
> > +     pfree(buf_text);
> > +
> > +     ret_text = PG_STR_GET_TEXT(str->data);
> > +     pfree(str->data);
> > +     pfree(str);
> > +     pfree(data);
> >
> >       PG_RETURN_TEXT_P(ret_text);
> >   }
> > *** ./src/include/catalog/pg_proc.h.orig    Tue Jul 20 09:24:11 2004
> > --- ./src/include/catalog/pg_proc.h    Tue Jul 20 09:26:11 2004
> > ***************
> > *** 2186,2191 ****
> > --- 2186,2193 ----
> >   DESCR("return portion of string");
> >   DATA(insert OID =  2087 ( replace       PGNSP PGUID 12 f f t f i 3 25 "25 25
> > 25"  replace_text - _null_ ));
> >   DESCR("replace all occurrences of old_substr with new_substr in string");
> > + DATA(insert OID =  2167 ( regexp_replace    PGNSP PGUID 12 f f t f i 3 25
> > "25 25 25"  textregexreplace - _null_ ));
> > + DESCR("replace text using regexp");
> >   DATA(insert OID =  2088 ( split_part   PGNSP PGUID 12 f f t f i 3 25 "25
> > 25 23"  split_text - _null_ ));
> >   DESCR("split string by field_sep and return field_num");
> >   DATA(insert OID =  2089 ( to_hex       PGNSP PGUID 12 f f t f i 1 25 "23"
> > to_hex32 - _null_ ));
> > *** ./src/include/regex/regex.h.orig    Tue Jul 20 08:51:06 2004
> > --- ./src/include/regex/regex.h    Tue Jul 20 08:51:16 2004
> > ***************
> > *** 163,169 ****
> >    * the prototypes for exported functions
> >    */
> >   extern int    pg_regcomp(regex_t *, const pg_wchar *, size_t, int);
> > ! extern int    pg_regexec(regex_t *, const pg_wchar *, size_t, rm_detail_t *,
> > size_t, regmatch_t[], int);
> >   extern void pg_regfree(regex_t *);
> >   extern size_t pg_regerror(int, const regex_t *, char *, size_t);
> >
> > --- 163,169 ----
> >    * the prototypes for exported functions
> >    */
> >   extern int    pg_regcomp(regex_t *, const pg_wchar *, size_t, int);
> > ! extern int    pg_regexec(regex_t *, const pg_wchar *, size_t, size_t,
> > rm_detail_t *, size_t, regmatch_t[], int);
> >   extern void pg_regfree(regex_t *);
> >   extern size_t pg_regerror(int, const regex_t *, char *, size_t);
> >
> > *** ./src/include/utils/builtins.h.orig    Tue Jul 20 09:11:19 2004
> > --- ./src/include/utils/builtins.h    Tue Jul 20 09:11:46 2004
> > ***************
> > *** 408,413 ****
> > --- 408,414 ----
> >   extern Datum texticregexeq(PG_FUNCTION_ARGS);
> >   extern Datum texticregexne(PG_FUNCTION_ARGS);
> >   extern Datum textregexsubstr(PG_FUNCTION_ARGS);
> > + extern Datum textregexreplace(PG_FUNCTION_ARGS);
> >   extern Datum similar_escape(PG_FUNCTION_ARGS);
> >   extern const char *assign_regex_flavor(const char *value,
> >                       bool doit, bool interactive);
> > ***************
> > *** 537,542 ****
> > --- 538,544 ----
> >   extern bool SplitIdentifierString(char *rawstring, char separator,
> >                         List **namelist);
> >   extern Datum replace_text(PG_FUNCTION_ARGS);
> > + extern Datum replace_text_regexp(PG_FUNCTION_ARGS);
> >   extern Datum split_text(PG_FUNCTION_ARGS);
> >   extern Datum text_to_array(PG_FUNCTION_ARGS);
> >   extern Datum array_to_text(PG_FUNCTION_ARGS);
> >
> > __________________________________________________
> > Do You Yahoo!?
> > http://bb.yahoo.co.jp/
> >
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 3: if posting/reading through Usenet, please send an appropriate
> >       subscribe-nomail command to majordomo@postgresql.org so that your
> >       message can get through to the mailing list cleanly
> >
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq

--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100   mobile: +1 415 235 3778

Remember to vote!

Re: [PATCHES] regexp_replace

From
Bruce Momjian
Date:
David Fetter wrote:
> On Mon, Jun 06, 2005 at 12:02:18PM -0400, Bruce Momjian wrote:
> >
> > Patch removed because we already have this functionality.
>
> We don't yet have this functionality, as the patch allows for using
> second and later regex matches "()" in the replacement pattern.
>
> The function is misnamed.  It should be called regex_replace_all() or
> some such, as it violates the principle of least astonishment by
> replacing all instances by default.  Every other regex replacement
> defaults to "replace first," not "replace all."  Or maybe it should
> take a bool for "replace all," or...?  Anyhow, it's worth a discussion
> :)

Does anyone want to argue that this additional functionality is
significant and deserves its own function or an additional argument to
the existing function?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [PATCHES] regexp_replace

From
a_ogawa
Date:
Bruce Momjian wrote:
> David Fetter wrote:
> > On Mon, Jun 06, 2005 at 12:02:18PM -0400, Bruce Momjian wrote:
> > >
> > > Patch removed because we already have this functionality.
> >
> > We don't yet have this functionality, as the patch allows for using
> > second and later regex matches "()" in the replacement pattern.
> >
> > The function is misnamed.  It should be called regex_replace_all() or
> > some such, as it violates the principle of least astonishment by
> > replacing all instances by default.  Every other regex replacement
> > defaults to "replace first," not "replace all."  Or maybe it should
> > take a bool for "replace all," or...?  Anyhow, it's worth a discussion
> > :)
>
> Does anyone want to argue that this additional functionality is
> significant and deserves its own function or an additional argument to
> the existing function?

Oracle10g has a similar functionality. The name is regexp_replace.
There is the following usages in this functionality.
- Format the ZIP code and the telephone number, etc.
   Example: select regexp_replace('1112223333', '(\\d{3})(\\d{3})(\\d{4})',
                                  '(\\1) \\2-\\3');
            result: (111) 222-3333
- Delete an unnecessary white space.
   Example: select regexp_replace('A     B     C', '\\s+', ' ');
            result: A B C

I think that the usage increases if "replace all" or "replace first" can be
specified to this function.

regards,

---
Atsushi Ogawa


Re: [PATCHES] regexp_replace

From
David Fetter
Date:
On Tue, Jun 07, 2005 at 09:35:56AM +0900, a_ogawa wrote:
>
> Bruce Momjian wrote:
> > David Fetter wrote:
> > > On Mon, Jun 06, 2005 at 12:02:18PM -0400, Bruce Momjian wrote:
> > > >
> > > > Patch removed because we already have this functionality.
> > >
> > > We don't yet have this functionality, as the patch allows for
> > > using second and later regex matches "()" in the replacement
> > > pattern.
> > >
> > > The function is misnamed.  It should be called
> > > regex_replace_all() or some such, as it violates the principle
> > > of least astonishment by replacing all instances by default.
> > > Every other regex replacement defaults to "replace first," not
> > > "replace all."  Or maybe it should take a bool for "replace
> > > all," or...?  Anyhow, it's worth a discussion :)
> >
> > Does anyone want to argue that this additional functionality is
> > significant and deserves its own function or an additional
> > argument to the existing function?
>
> Oracle10g has a similar functionality. The name is regexp_replace.
> There is the following usages in this functionality.
> - Format the ZIP code and the telephone number, etc.
>    Example: select regexp_replace('1112223333', '(\\d{3})(\\d{3})(\\d{4})',
>                                   '(\\1) \\2-\\3');
>             result: (111) 222-3333
> - Delete an unnecessary white space.
>    Example: select regexp_replace('A     B     C', '\\s+', ' ');
>             result: A B C
>
> I think that the usage increases if "replace all" or "replace first" can be
> specified to this function.

Ogawa-san,

I think that this would be a case for function overloading:

function regexp_replace(
    string text, pattern text, replacement text
) RETURNS TEXT; /* First only */

regexp_replace(
    string text, pattern text, replacement text, global bool
) RETURNS TEXT; /* Global if global is TRUE, first only otherwise */

What do you think of this idea?  One trouble is that there are some
other options.  For example, one could add switches for all
combinations of "global," "case insensitive," "compile once," "exclude
whitespace," etc. as perl does.  Do we want to go this route?

Cheers,
D
--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100   mobile: +1 415 235 3778

Remember to vote!

Re: [PATCHES] regexp_replace

From
Atsushi Ogawa
Date:
David Fetter wrote:
> On Tue, Jun 07, 2005 at 09:35:56AM +0900, a_ogawa wrote:
> > David Fetter wrote:
> > > We don't yet have this functionality, as the patch allows for
> > > using second and later regex matches "()" in the replacement
> > > pattern.
> > >
> > > The function is misnamed.  It should be called
> > > regex_replace_all() or some such, as it violates the principle
> > > of least astonishment by replacing all instances by default.
> > > Every other regex replacement defaults to "replace first," not
> > > "replace all."  Or maybe it should take a bool for "replace
> > > all," or...?  Anyhow, it's worth a discussion :)
> >
> > I think that the usage increases if "replace all" or "replace first" can
be
> > specified to this function.
>
> Ogawa-san,
>
> I think that this would be a case for function overloading:
>
> function regexp_replace(
>     string text, pattern text, replacement text
> ) RETURNS TEXT; /* First only */
>
> regexp_replace(
>     string text, pattern text, replacement text, global bool
> ) RETURNS TEXT; /* Global if global is TRUE, first only otherwise */
>
> What do you think of this idea?  One trouble is that there are some
> other options.  For example, one could add switches for all
> combinations of "global," "case insensitive," "compile once," "exclude
> whitespace," etc. as perl does.  Do we want to go this route?

My idea is opposite. I think that the regexp_replace() should make
"replace all" a default. Because the replace() of pgsql replaces all
string, and regexp_replace() of oracle10g is also similar.

And I think that it is better to be able to specify the option with text.
I think about this function specification:

regexp_replace(
    string text, pattern text, replacement text
) RETURNS TEXT; /* Replace all */

regexp_replace(
    string text, pattern text, replacement text, options text
) RETURNS TEXT; /* Change operation by the option. */

The options can use the following values.
 f: Replace first only
 i: Case insensitive

Any comments?

regards,

---
Atsushi Ogawa


Re: [PATCHES] regexp_replace

From
David Fetter
Date:
On Tue, Jun 07, 2005 at 10:27:28PM +0900, Atsushi Ogawa wrote:
>
> David Fetter wrote:
> > On Tue, Jun 07, 2005 at 09:35:56AM +0900, a_ogawa wrote:
> > > David Fetter wrote:
> > > > We don't yet have this functionality, as the patch allows for
> > > > using second and later regex matches "()" in the replacement
> > > > pattern.
> > > >
> > > > The function is misnamed.  It should be called
> > > > regex_replace_all() or some such, as it violates the principle
> > > > of least astonishment by replacing all instances by default.
> > > > Every other regex replacement defaults to "replace first," not
> > > > "replace all."  Or maybe it should take a bool for "replace
> > > > all," or...?  Anyhow, it's worth a discussion :)
> > >
> > > I think that the usage increases if "replace all" or "replace first" can
> be
> > > specified to this function.
> >
> > Ogawa-san,
> >
> > I think that this would be a case for function overloading:
> >
> > function regexp_replace(
> >     string text, pattern text, replacement text
> > ) RETURNS TEXT; /* First only */
> >
> > regexp_replace(
> >     string text, pattern text, replacement text, global bool
> > ) RETURNS TEXT; /* Global if global is TRUE, first only otherwise */
> >
> > What do you think of this idea?  One trouble is that there are some
> > other options.  For example, one could add switches for all
> > combinations of "global," "case insensitive," "compile once," "exclude
> > whitespace," etc. as perl does.  Do we want to go this route?
>
> My idea is opposite. I think that the regexp_replace() should make
> "replace all" a default. Because the replace() of pgsql replaces all
> string, and regexp_replace() of oracle10g is also similar.

I respectfully disagree.  Although Oracle does things this way, no
other regular expression search and replace does.  Historically, you
can find that "Oracle does it this way" is not a reason why we would
do it.  Text editors, programming languages, etc., etc. do "replace
the first" by default and "replace globally" only when told to.

> And I think that it is better to be able to specify the option with text.
> I think about this function specification:
>
> regexp_replace(
>     string text, pattern text, replacement text
> ) RETURNS TEXT; /* Replace all */
>
> regexp_replace(
>     string text, pattern text, replacement text, options text
> ) RETURNS TEXT; /* Change operation by the option. */
>
> The options can use the following values.
>  f: Replace first only
>  i: Case insensitive
>
> Any comments?

I think that "case insensitive" is a good thing to add separately as a
boolean :)

Cheers,
David.
--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100   mobile: +1 415 235 3778

Remember to vote!

Re: [PATCHES] regexp_replace

From
Atsushi Ogawa
Date:
David Fetter wrote:
> On Tue, Jun 07, 2005 at 10:27:28PM +0900, Atsushi Ogawa wrote:
> > David Fetter wrote:
> > > Ogawa-san,
> > >
> > > I think that this would be a case for function overloading:
> > >
> > > function regexp_replace(
> > >     string text, pattern text, replacement text
> > > ) RETURNS TEXT; /* First only */
> > >
> > > regexp_replace(
> > >     string text, pattern text, replacement text, global bool
> > > ) RETURNS TEXT; /* Global if global is TRUE, first only otherwise */
> > >
> > > What do you think of this idea?  One trouble is that there are some
> > > other options.  For example, one could add switches for all
> > > combinations of "global," "case insensitive," "compile once," "exclude
> > > whitespace," etc. as perl does.  Do we want to go this route?
> >
> > My idea is opposite. I think that the regexp_replace() should make
> > "replace all" a default. Because the replace() of pgsql replaces all
> > string, and regexp_replace() of oracle10g is also similar.
>
> I respectfully disagree.  Although Oracle does things this way, no
> other regular expression search and replace does.  Historically, you
> can find that "Oracle does it this way" is not a reason why we would
> do it.  Text editors, programming languages, etc., etc. do "replace
> the first" by default and "replace globally" only when told to.

I agree. Let's stop the way to Oracle.

> > And I think that it is better to be able to specify the option with
text.
>
> I think that "case insensitive" is a good thing to add separately as a
> boolean :)

I don't like to specify operation with boolean. ;)
Because when a SQL is read, the meaning becomes indistinct.

How about changing the function name of each usage?
regexp_replace:        replace firstregexp_replace_all:    replace allregexp_ic_replace:     replace first and case
insensitiveregexp_ic_replace_all:replace all and case insensitive
 

regards,

--
Atsushi Ogawa



Re: [PATCHES] regexp_replace

From
Hannu Krosing
Date:
On K, 2005-06-08 at 21:32 +0900, Atsushi Ogawa wrote:

> 
> How about changing the function name of each usage?
> 
>  regexp_replace:        replace first
>  regexp_replace_all:    replace all
>  regexp_ic_replace:     replace first and case insensitive
>  regexp_ic_replace_all: replace all and case insensitive

perhaps just "regexp_ireplace", we already have ILIKE not IC_LIKE


-- 
Hannu Krosing <hannu@skype.net>



Re: [PATCHES] regexp_replace

From
Robert Treat
Date:
On Tuesday 07 June 2005 10:57, David Fetter wrote:
> On Tue, Jun 07, 2005 at 10:27:28PM +0900, Atsushi Ogawa wrote:
> > My idea is opposite. I think that the regexp_replace() should make
> > "replace all" a default. Because the replace() of pgsql replaces all
> > string, and regexp_replace() of oracle10g is also similar.
>
> I respectfully disagree.  Although Oracle does things this way, no
> other regular expression search and replace does.  Historically, you
> can find that "Oracle does it this way" is not a reason why we would
> do it.  Text editors, programming languages, etc., etc. do "replace
> the first" by default and "replace globally" only when told to.
>

You don't think it will be confusing to have a function called replace which
replaces all occurrences and a function called regex_replace which only
replaces the first occurance?  There's something to be said for consitancy
within pgsql itself.

--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

Re: [PATCHES] regexp_replace

From
Bruce Momjian
Date:
Robert Treat wrote:
> On Tuesday 07 June 2005 10:57, David Fetter wrote:
> > On Tue, Jun 07, 2005 at 10:27:28PM +0900, Atsushi Ogawa wrote:
> > > My idea is opposite. I think that the regexp_replace() should make
> > > "replace all" a default. Because the replace() of pgsql replaces all
> > > string, and regexp_replace() of oracle10g is also similar.
> >
> > I respectfully disagree.  Although Oracle does things this way, no
> > other regular expression search and replace does.  Historically, you
> > can find that "Oracle does it this way" is not a reason why we would
> > do it.  Text editors, programming languages, etc., etc. do "replace
> > the first" by default and "replace globally" only when told to.
> >
>
> You don't think it will be confusing to have a function called replace which
> replaces all occurrences and a function called regex_replace which only
> replaces the first occurance?  There's something to be said for consitancy
> within pgsql itself.

Huh?  I am confused.  Why if both support regex, why does regex_replace
only do the first one?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [PATCHES] regexp_replace

From
"Andrew Dunstan"
Date:
Atsushi Ogawa said:
>
> How about changing the function name of each usage?
>
> regexp_replace:        replace first
> regexp_replace_all:    replace all
> regexp_ic_replace:     replace first and case insensitive
> regexp_ic_replace_all: replace all and case insensitive
>


That's just horrible. The number of function names will double with each
supported flag (e.g. I'd like to see extended regexes supported).

surely somthing like
 foo_replace (source text, pattern text, replacement text, flags text)
returns text

would fit the bill. OK, it reflects my Perl prejudices, but that looks more
natural to me. You could overload it so that the flags default to none
(which would be case sensitive, replace the first instance only, among other
things).

cheers

andrew




Re: [PATCHES] regexp_replace

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> Atsushi Ogawa said:
> >
> > How about changing the function name of each usage?
> >
> > regexp_replace:        replace first
> > regexp_replace_all:    replace all
> > regexp_ic_replace:     replace first and case insensitive
> > regexp_ic_replace_all: replace all and case insensitive
> >
> 
> 
> That's just horrible. The number of function names will double with each
> supported flag (e.g. I'd like to see extended regexes supported).
> 
> surely somthing like
> 
>   foo_replace (source text, pattern text, replacement text, flags text)
> returns text
> 
> would fit the bill. OK, it reflects my Perl prejudices, but that looks more
> natural to me. You could overload it so that the flags default to none
> (which would be case sensitive, replace the first instance only, among other
> things).

Can we have the flags be a list of words, e.g.  "all, ignorecase".  How
do we handle this type of problem in other cases?

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: [PATCHES] regexp_replace

From
Tom Flavel
Date:
On 08/06/2005 21:57:29, Bruce Momjian wrote:
> Andrew Dunstan wrote:
> > Atsushi Ogawa said:
> > >
> > > How about changing the function name of each usage?
> > >
> > > regexp_replace:        replace first
> > > regexp_replace_all:    replace all
> > > regexp_ic_replace:     replace first and case insensitive
> > > regexp_ic_replace_all: replace all and case insensitive
> > 
> > 
> > That's just horrible. The number of function names will double with each
> > supported flag (e.g. I'd like to see extended regexes supported).
> > 
> > surely somthing like
> > 
> >   foo_replace (source text, pattern text, replacement text, flags text)
> > returns text
> > 
> > would fit the bill. OK, it reflects my Perl prejudices, but that looks more
> > natural to me. You could overload it so that the flags default to none
> > (which would be case sensitive, replace the first instance only, among other
> > things).
> 
> Can we have the flags be a list of words, e.g.  "all, ignorecase".  How
> do we handle this type of problem in other cases?

How about an array?

Cumbersome, perhaps, but it makes more sense to me than delimiting with
commas or using single characters.

-- 
Tom


Re: [PATCHES] regexp_replace

From
Atsushi Ogawa
Date:
Tom Flavel wrote:
> On 08/06/2005 21:57:29, Bruce Momjian wrote:
> > Andrew Dunstan wrote:
> > >
> > > surely somthing like
> > >
> > >   foo_replace (source text, pattern text, replacement text, flags
text)
> > > returns text
> > >
> > > would fit the bill. OK, it reflects my Perl prejudices, but that
> > > looks more natural to me. You could overload it so that the flags
> > > default to none
> > > (which would be case sensitive, replace the first instance only,
> > > among other things).
> >
> > Can we have the flags be a list of words, e.g.  "all, ignorecase".  How
> > do we handle this type of problem in other cases?
>
> How about an array?
>
> Cumbersome, perhaps, but it makes more sense to me than delimiting with
> commas or using single characters.

I think that it is good to specify the flags by one character as well
as Perl.

I propose the following specification:

regexp_replace(source text, pattern text, replacement text, [flags text])
returns text

The flags can use the following values:g: global (replace all)i: ignore case

When the flags is not specified, case sensitive, replace the first
instance only.

regards,

---
Atsushi Ogawa



Re: [PATCHES] regexp_replace

From
Bruce Momjian
Date:
Atsushi Ogawa wrote:
> I think that it is good to specify the flags by one character as well
> as Perl.
> 
> I propose the following specification:
> 
> regexp_replace(source text, pattern text, replacement text, [flags text])
> returns text
> 
> The flags can use the following values:
>  g: global (replace all)
>  i: ignore case
> 
> When the flags is not specified, case sensitive, replace the first
> instance only.

This seems good to me.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: [PATCHES] regexp_replace

From
Atsushi Ogawa
Date:
Bruce Momjian wrote:
> Atsushi Ogawa wrote:
> > I propose the following specification:
> >
> > regexp_replace(source text, pattern text, replacement text, [flags
text])
> > returns text
> >
> > The flags can use the following values:
> >  g: global (replace all)
> >  i: ignore case
> >
> > When the flags is not specified, case sensitive, replace the first
> > instance only.
>
> This seems good to me.

Thanks. I will remake the patch within a few days.

regards,

--- Atsushi Ogawa