Re: Row pattern recognition - Mailing list pgsql-hackers

From Chao Li
Subject Re: Row pattern recognition
Date
Msg-id FA19814A-6ADF-4137-8BB4-D5736598EF69@gmail.com
Whole thread Raw
In response to Re: Row pattern recognition  (Chao Li <li.evan.chao@gmail.com>)
List pgsql-hackers

> On Nov 20, 2025, at 15:33, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
> I’d stop here, and continue 0005 tomorrow.
>

I reviewed 0005 today. I'm still not very familiar with the executor code, so going through 0005 has also been a
valuablelearning process for me. 

12 - 0005 - nodeWindowAgg.c
```
+    if (string_set_get_size(str_set) == 0)
+    {
+        /* no match found in the first row */
+        register_reduced_frame_map(winstate, original_pos, RF_UNMATCHED);
+        destroyStringInfo(encoded_str);
+        return;
+    }
```

encoded_str should also be destroyed if not entering the “if” clause.

13 - 0005 - nodeWindowAgg.c
```
static
int
do_pattern_match(char *pattern, char *encoded_str, int len)
{
static regex_t *regcache = NULL;
static regex_t preg;
static char patbuf[1024]; /* most recent 'pattern' is cached here */
```

Using static variable in executor seems something I have never seen, which may persistent data across queries. I think
usuallyper query data are stored in state structures. 

14 - 0005 - nodeWindowAgg.c
```
+        MemoryContext oldContext = MemoryContextSwitchTo(TopMemoryContext);
+
+        if (regcache != NULL)
+            pg_regfree(regcache);    /* free previous re */
+
+        /* we need to convert to char to pg_wchar */
+        plen = strlen(pattern);
+        data = (pg_wchar *) palloc((plen + 1) * sizeof(pg_wchar));
+        data_len = pg_mb2wchar_with_len(pattern, data, plen);
+        /* compile re */
+        sts = pg_regcomp(&preg, /* compiled re */
+                         data,    /* target pattern */
+                         data_len,    /* length of pattern */
+                         cflags,    /* compile option */
+                         C_COLLATION_OID    /* collation */
+            );
+        pfree(data);
+
+        MemoryContextSwitchTo(oldContext);
```

Here in do_pattern_match, it switches to top memory context and compiles the re and stores to “preg". Based on the
commentof pg_regcomp: 
```
/*
* pg_regcomp - compile regular expression
*
* Note: on failure, no resources remain allocated, so pg_regfree()
* need not be applied to re.
*/
int
pg_regcomp(regex_t *re,
const chr *string,
size_t len,
int flags,
Oid collation)
```

“preg” should be freed by pg_regfree(), given the memory is allocated in TopMemoryContext, this looks like a memory
leak.

Okay, I’d stop here and continue to review 0006 next week.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: How can end users know the cause of LR slot sync delays?
Next
From: shveta malik
Date:
Subject: Re: How can end users know the cause of LR slot sync delays?