Thread

Re: Row pattern recognition

Tatsuo Ishii <ishii@postgresql.org> — 2025-11-27T03:10:08Z

Hi Chao,

Any comment on this?

>> 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.
>
>Yeah, such a usage maybe rare. But I hesitate to store the data
>(regex_t) in WindowAggState because:
>
>(1) it bloats WindowAggState which we want to avoid if possible
>(2) it requires execnodes.h to include regex.h. (maybe this itself is harmless, I am not sure)
>
>> 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.
>
>I admit current patch leaves the memory unfreed even after a query
>finishes. What about adding something like:
>
>static void do_pattern_match_end(void)
>{
>	if (regcache != NULL)
>		pg_regfree(regcache);
>}
>
>and let ExecEndWindowAgg() call it?

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp