On 25.08.2023 8:57 PM, Jelte Fennema wrote:
The cached plan for a prepared statements can get invalidated when DDL
changes the tables used in the query, or when search_path changes. When
this happens the prepared statement can still be executed, but it will
be replanned in the new context. This means that the prepared statement
will do something different e.g. in case of search_path changes it will
select data from a completely different table. This won't throw an
error, because it is considered the responsibility of the operator and
query writers that the query will still do the intended thing.
However, we would throw an error if the the result of the query is of a
different type than it was before:
ERROR: cached plan must not change result type
This requirement was not documented anywhere and it
can thus be a surprising error to hit. But it's actually not needed for
this to be an error, as long as we send the correct RowDescription there
does not have to be a problem for clients when the result types or
column counts change.
This patch starts to allow a prepared statement to continue to work even
when the result type changes.
Without this change all clients that automatically prepare queries as a
performance optimization will need to handle or avoid the error somehow,
often resulting in deallocating and re-preparing queries when its
usually not necessary. With this change connection poolers can also
safely prepare the same query only once on a connection and share this
one prepared query across clients that prepared that exact same query.
Some relevant previous discussions:
[1]:
https://www.postgresql.org/message-id/flat/CAB%3DJe-GQOW7kU9Hn3AqP1vhaZg_wE9Lz6F4jSp-7cm9_M6DyVA%40mail.gmail.com
[2]:
https://stackoverflow.com/questions/2783813/postgres-error-cached-plan-must-not-change-result-type
[3]:
https://stackoverflow.com/questions/42119365/how-to-avoid-cached-plan-must-not-change-result-type-error
[4]: https://github.com/pgjdbc/pgjdbc/pull/451
[5]: https://github.com/pgbouncer/pgbouncer/pull/845#discussion_r1305295551
[6]: https://github.com/jackc/pgx/issues/927
[7]:
https://elixirforum.com/t/postgrex-errors-with-cached-plan-must-not-change-result-type-during-migration/51235/2
[8]: https://github.com/rails/rails/issues/12330
The following assignment of format is not corrects:
/* Do nothing if portal won't return tuples */
if (portal->tupDesc == NULL)
+ {
+ /*
+ * For SELECT like queries we delay filling in the tupDesc
until after
+ * PortalRunUtility, because we don't know what rows an EXECUTE
+ * command will return. Because we delay setting tupDesc, we
also need
+ * to delay setting formats. We do this in a pretty hacky way, by
+ * temporarily setting the portal formats to the passed in formats.
+ * Then once we fill in tupDesc, we call PortalSetResultFormat
again
+ * with portal->formats to fill in the final formats value.
+ */
+ if (portal->strategy == PORTAL_UTIL_SELECT)
+ portal->formats = formats;
return;
because it is create in other memory context:
postgres.c:
/* Done storing stuff in portal's context */
MemoryContextSwitchTo(oldContext);
...
/* Get the result format codes */
numRFormats = pq_getmsgint(input_message, 2);
if (numRFormats > 0)
{
rformats = palloc_array(int16, numRFormats);
for (int i = 0; i < numRFormats; i++)
rformats[i] = pq_getmsgint(input_message, 2);
}
It has to be copied as below:
portal->formats = (int16 *)
MemoryContextAlloc(portal->portalContext,
natts * sizeof(int16));
memcpy(portal->formats, formats, natts * sizeof(int16));
or alternatively MemoryContextSwitchTo(oldContext) should be moved
after initialization of rformat
