Re: fill_extraUpdatedCols is done in completely the wrong place
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2020-05-18T17:57:19Z
Lists: pgsql-hackers
Attachments
- broken-update-rule.sql (text/plain)
- broken-updatable-views.sql (text/plain)
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: > On 2020-05-08 21:05, Tom Lane wrote: >> I happened to notice $subject while working on the release notes. >> AFAICS, it is 100% inappropriate for the parser to compute the >> set of generated columns affected by an UPDATE, because that set >> could change before execution. It would be really easy to break >> this for an UPDATE in a stored rule, for example. > Do you have a specific situation in mind? How would a rule change the > set of columns updated by a query? Something involving CTEs? Having a > test case would be good. broken-update-rule.sql, attached, shows the scenario I had in mind: the rule UPDATE query knows nothing of the generated column that gets added after the rule is stored, so the UPDATE fails to update it. However, on the way to preparing that test case I discovered that auto-updatable views have the same disease even when the generated column exists from the get-go; see broken-updatable-views.sql. In the context of the existing design, I suppose this means that there needs to be a fill_extraUpdatedCols call somewhere in the code path that constructs an auto-update query. But if we moved the whole thing to the executor then the problem would go away. I observe also that the executor doesn't seem to need this bitmap at all unless (a) there are triggers or (b) there are generated columns. So in a lot of simpler cases, the cost of doing fill_extraUpdatedCols at either parse or plan time would be quite wasted. That might be a good argument for moving it to executor start, even though we'd then have to re-do it when re-using a prepared plan. regards, tom lane
Commits
-
Calculate extraUpdatedCols in query rewriter, not parser.
- ad77039fad0f 14.0 landed
- 70492195be5e 13.1 landed
- 43330cdd40f1 12.5 landed