| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Vik Fearing <vik(at)postgresfriends(dot)org>, Tatsuo Ishii <ishii(at)postgresql(dot)org> |
| Cc: | david(dot)g(dot)johnston(at)gmail(dot)com, jacob(dot)champion(at)enterprisedb(dot)com, er(at)xs4all(dot)nl, peter(at)eisentraut(dot)org, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Row pattern recognition |
| Date: | 2025-11-21 05:25:50 |
| Message-ID: | FA19814A-6ADF-4137-8BB4-D5736598EF69@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Nov 20, 2025, at 15:33, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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 valuable learning 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 usually per 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 comment of 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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2025-11-21 05:30:36 | Re: How can end users know the cause of LR slot sync delays? |
| Previous Message | Amit Kapila | 2025-11-21 04:28:37 | Re: How can end users know the cause of LR slot sync delays? |