Re: BUG #18097: Immutable expression not allowed in generated at

Aleksander Alekseev <aleksander@timescale.com>

From: Aleksander Alekseev <aleksander@timescale.com>

To: pgsql-hackers@lists.postgresql.org

Cc: Tom Lane <tgl@sss.pgh.pa.us>, James Keener <jim@jimkeener.com>

Date: 2023-11-14T12:10:05Z

Lists: pgsql-bugs, pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Ensure we preprocess expressions before checking their volatility.
- f07a3039c72b 16.2 landed
- abd1b1325d60 12.18 landed
- 9bd0f74eac19 14.11 landed
- 9057ddbefe7e 15.6 landed
- 743ddafc7124 17.0 landed
- 5c11104c7d20 13.14 landed

Hi,

I noticed that the patchset needs a review and decided to take a look.

> There are a couple of points worth bikeshedding perhaps.  I didn't
> spend much thought on the wrapper functions' names, but it's surely
> true that the semantic difference between contain_mutable_functions
> and ContainMutableFunctions is quite un-apparent from those names.
> Anybody got a better idea?

Oh no! We encountered one of the most difficult problems in computer
science [1].

ContainMutableFunctionsAfterPerformingPlannersTransformations() would
be somewhat long but semantically correct. It can be shortened to
ContainMutableFunctionsAfterTransformations() or perhaps
TransformedExprContainMutableFunctions(). Personally I don't mind long
names. This being said, ContainMutableFunctions() doesn't disgusts my
sense of beauty too much either. All in all any name will do IMO.
Naturally ContainVolatileFunctions() should be renamed consistently
with ContainMutableFunctions().

I couldn't find anything wrong with 0001..0003. The parches were
tested in several environments and passed `make check-world`. I
suggest merging them.

[1]: https://martinfowler.com/bliki/TwoHardThings.html

-- 
Best regards,
Aleksander Alekseev