[mgaffigan]

Postgres would have intermittent 500 errors after a metadata column changed from one type to another. Fix that.

Closes #458
Caught during #449

[github-actions]

Test Results

129 files  ±0  129 suites  ±0   2m 54s ⏱️ +13s
737 tests ±0  737 ✅ ±0  0 💤 ±0  0 ❌ ±0 
827 runs  ±0  821 ✅ ±0  6 💤 ±0  0 ❌ ±0 

Results for commit 2ad1100. ± Comparison against base commit fbda474.

[tonygermano]

Thanks for tracking this down. Targeting the two SELECT * metadata queries is a reasonable, contained fix for #458. The changes I'm requesting are in the inline comments. Nothing below is blocking; these are suggestions and questions, roughly in order of importance.

Tests

This PR doesn't add a test, but CustomMetaDataColumnRedeployTest in #449 covers the scenario end to end, and that PR's CI runs it against both PostgreSQL configurations. Once #449 lands, that should catch a regression if a later refactor drops the disableServerSidePlanCache call.

Does that test fail every time without this fix, or only sometimes? pgjdbc only switches to a named server-side statement after the same query has run 5 times (its default prepareThreshold) on the same connection. The test reads the metadata only a few times before the redeploy, and the pool has several connections. If it is intermittent, a few extra metadata reads before the redeploy should make it fail consistently without the fix.

Force-binary connections

As I understand pgjdbc's PgStatement, a statement is only treated as one-shot (unnamed) when the threshold allows it and force-binary transfer is off. setPrepareThreshold(0) doesn't clear the force-binary flag. That means a connection URL with prepareThreshold=-1 or forceBinary=true would still get a named server-side statement, and the "cached plan must not change result type" error would still happen. These are uncommon settings. If you can confirm the behaviour, a note in the javadoc would help.

Scope of the fix

This fixes the two queries that are known to break, but the underlying cause is more general. pgjdbc keeps named prepared statements per connection, and those statements outlive DDL on our pooled connections. Any other query whose result shape depends on D_MCMn columns could hit the same error after a metadata column is added, removed or retyped. That includes queries added later and queries from extensions. Some connection-level options would cover all of them:

• autosave=conservative as a default for the PostgreSQL pool. pgjdbc then retries exactly this error after re-preparing the statement.
• Evicting pooled connections, or running DEALLOCATE ALL, after addMetaDataColumn / removeMetaDataColumn.

I'm fine with the targeted approach for this PR. Do you think one of these is worth a follow-up issue?

Performance

With a threshold of 0, these two queries are parsed and planned on every execution and never switch to a named statement. getMetaDataMap runs once per message when browsing and searching messages, so this adds some overhead to a frequently used path to handle a rare schema change. I expect it's small next to the rest of the search, so I'm not asking for changes here. It's one more reason the invalidate-after-DDL options above might be the better long-term design.

---

🤖 Drafted with Claude Code

[tonygermano]

chore: fix file header

[tonygermano]

Both Claude and Gemini suggested adding this guard for safety against a non-pgjdbc driver.

Suggested change

	statement.unwrap(PGStatement.class).setPrepareThreshold(0);
	if (statement.isWrapperFor(PGStatement.class)) {
	statement.unwrap(PGStatement.class).setPrepareThreshold(0);
	}

[tonygermano]

Claude:

Name and javadoc describe the wrong thing. prepareThreshold=0 stops pgjdbc from using a named server-side prepared statement, along with its cached result description. It doesn't really "disable plan caching." Something like useUnnamedStatement or disableServerPrepare would keep a future Oracle or SQL Server override from implementing the wrong behaviour. The existing // do not cache this statement comment above each call now means two different things.

I had to look it up to see what prepareThreshold did, but specifies the limit where if the driver uses a client-side preparedStatement threshold number of times, then it creates a server-side prepared statement to use for future calls. Setting it to 0 instructs the client to not create the server-side prepare.