Skip to content

Gate the two changes from #2 that broke WarehousePG 6 and 7 - #5

Merged
Bonartze merged 1 commit into
whpgfrom
whpgn-704-fix-pr2-regressions
Sep 15, 2026
Merged

Bonartze merged 1 commit into
whpgfrom
whpgn-704-fix-pr2-regressions

Conversation

@Bonartze

@Bonartze Bonartze commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Fixes only what #2 broke. Nothing else — the CI lane, the WHPG19 conf-install
defect and the upstream-sync assessment follow in a second PR.

The problem

#2 adapted pljava-so for PG19 and got WHPG19 from a hard build failure to
7/7, which it did. But two changes went in ungated, and the shared whpg
branch stopped working on both older server lines
from that merge
(2026-07-29) until now. Nothing in this repo builds it on any line, so there
was no signal.

Both changes were justified from upstream PostgreSQL history, and both
justifications are correct about upstream. The trap is that upstream version
numbers do not map onto WarehousePG's lines — WarehousePG 7 is PG12 and
WarehousePG 6 is PG9.4
, both older than the reasoning assumed.

1. SPITupleTable.numvals — build failure on WHPG7 and WHPG6

tts->alloced - tts->free became tts->numvals, on the grounds that numvals
"predates pljava's PG12 floor". It does not: it arrives in PostgreSQL 13
(upstream "Further adjust SPITupleTable to provide a public row-count field"),
which also removed free and repurposed alloced as the private array length.

numvals alloced free
PG9.4 (WHPG6), PG12 (WHPG7) absent # alloced vals # free vals
PG13+ public row count private array length removed
error: 'SPITupleTable' has no member named 'numvals'; did you mean 'vals'?

Now selected on PG_VERSION_NUM >= 130000.

2. SPI_restore_connection() — WHPG6 broken at run time

The call was deleted from SPI_rollbackSavepoint as "a documented no-op since
PostgreSQL 10". True from PG10 on, which is exactly why it is invisible on
WHPG7 and WHPG19 — PG12 has it as #define SPI_restore_connection() ((void) 0).
WHPG6 is PG9.4, where it is still a real function that re-establishes the SPI
connection after the subtransaction is aborted.

Verified against the headers rather than assumed:

Tree SPI_restore_connection
warehouse-pg @ WHPG_6X_STABLE extern void SPI_restore_connection(void); — real function
upstream ≤ REL9_6_STABLE real function
REL_10_STABLE … REL_16_STABLE #define … ((void) 0)
REL_17_STABLE removed outright

Without it, javatest.testTransactionRecovery() on WHPG6 fails and takes the
rest of the test with it:

ERROR:  java.sql.SQLException: SPI function SPI_prepare failed with error SPI_ERROR_UNCONNECTED
ERROR:  current transaction is aborted, commands ignored until end of transaction block

Now gated PG_VERSION_NUM < 100000, so it is emitted only where it does real
work and never on PG17+, where the symbol no longer exists.

This one compiles cleanly on every line. Only a run against a real WHPG6
cluster finds it — which is why it survived a header-level audit of #2 that
caught the numvals break.

Evidence

whpg-extensions-packaging, el9/amd64, cloudsmith-upload=false. Note that
smoke: is skipped, not failed, when package: dies — so the base branch
reads as green jobs plus skips while no test ever ran.

Branch Line package: smoke: Run
whpg (base) WHPG7 fail — numvals skipped 34949889099
whpg (base) WHPG6 fail — numvals skipped 34949885047
this branch WHPG7 success success 34952582722
this branch WHPG6 success success 34952576326

The same two fixes already passed both lines as part of the combined branch —
WHPG7 34950265220
and WHPG6 34950261266,
package: and smoke: green on both. The runs above re-confirm them in
isolation — package: and smoke: green on both lines.

WHPG19 is not a packaging target, so it was checked locally against
/usr/local/whpg/bin/pg_config (PG19beta1, GP 19.0.0) on a 3-segment cluster:
7/7, no regression.diffs — i.e. these gates do not regress the line #2
was written for.

Scope

2 files, +25/−4. Deliberately excluded, to follow separately:

  • the reproducible build image and WHPG19 regression lane (the reason this was
    invisible for two months);
  • Makefile installconfs gating on GP_MAJORVERSION == 7, which means
    30-pljava.conf is never installed on WHPG19 and is absent from the WHPG19
    gppkg payload;
  • the gpdb/packaging/Makefile instance of the same == 7 bug;
  • floor()/rint() used without <math.h> (pre-existing; GCC 14 and C23
    reject it);
  • the upstream 1.6.x sync assessment, including the Java 8 question that needs
    product sign-off.

Supersedes #3 and #4, both closed in favour of this split.

#2 adapted pljava-so for PG19 and got WHPG19 from a hard build failure to
7/7, but left two changes ungated. Both were justified from upstream
PostgreSQL history, and both justifications are right about upstream -
except upstream version numbers do not map onto WarehousePG lines.
WarehousePG 7 is PG12 and WarehousePG 6 is PG9.4, both older than the
reasoning assumed.

TupleTable.c: tts->alloced - tts->free became tts->numvals, on the grounds
that numvals predates the PG12 floor. It does not - numvals arrives in
PostgreSQL 13 ("Further adjust SPITupleTable to provide a public row-count
field"), which also removed free and repurposed alloced as the private
array length. PG9.4 and PG12 both still have the alloced/free pair, so the
shared whpg branch stopped compiling on both older lines:

  error: SPITupleTable has no member named numvals; did you mean vals?

Now selected on PG_VERSION_NUM >= 130000.

SPI.c: the SPI_restore_connection() call was deleted from
SPI_rollbackSavepoint as a documented no-op since PostgreSQL 10. That is
true from PG10 on, which is why it is invisible on WarehousePG 7 and
WHPG19 - PG12 has it as #define SPI_restore_connection() ((void) 0).
WarehousePG 6 is PG9.4, where it is still a real function that
re-establishes the SPI connection after the subtransaction is aborted.
Verified against the headers: extern void SPI_restore_connection(void) in
WHPG_6X_STABLE and upstream through REL9_6_STABLE, the no-op macro from
REL_10_STABLE, removed outright in REL_17_STABLE.

Without it, javatest.testTransactionRecovery() on WHPG6 fails with
SPI_ERROR_UNCONNECTED and every later statement aborts. This one compiles
cleanly everywhere, so only a run against a WHPG6 cluster finds it.

Now gated on PG_VERSION_NUM < 100000, so it is emitted only where it does
real work and never on PG17+, where the symbol no longer exists.

@24nishant 24nishant left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified : both upstream hashes are exact (1833f1a1c3b0, 75680c3d805e), the PG13 numvals/alloced/free account is right, and both gate boundaries land correctly — PG10–16 get no call since the no-op macro makes emitting it identical to not, PG17+ get none since the symbol is gone.

Two things not visible in the diff:

  • PG_VERSION_NUM does reach SPI.c, via pljava/SPI.h → PgObject.h → JNICalls.h → pljava.h. Worth confirming, since an undefined macro there would evaluate to 0 and emit the call on PG19 too.
  • Both #else branches restore the pre-#2 code byte-identically, including keeping SPI_restore_connection() outside the if block.

Also spot-checked the other 15 files #2 touched — object_aclcheck and funcname were already gated, bits8→uint8 is a typedef no-op, procsignal_sigusr1_handler is in 9.4. So these two are the only ungated ones, which the green package: + smoke: backs up.

One question on sequencing: the root cause is that nothing here builds the older lines, and the CI lane is deferred — so until it lands, the next break of this class is equally invisible. Any reason not to land it alongside this? Do we have ticket to track this?

Good catch on smoke: being skipped rather than failed when package: dies — worth fixing in the CI PR itself.

LGTM.

@Bonartze
Bonartze merged commit 1dbd775 into whpg Sep 15, 2026
@Bonartze
Bonartze deleted the whpgn-704-fix-pr2-regressions branch September 15, 2026 12:59
Bonartze added a commit that referenced this pull request Sep 15, 2026
#5 landed as a squash, so this branch was not an ancestor of whpg and its
copy of TupleTable.c would have reverted the comment that merged - the
narrower "WarehousePG 7 is PG12" wording, rather than naming both older
lines. Only a comment, but it re-narrows exactly the misconception that
caused the WHPG6 break, so the conflict is resolved to whpg on both gate
files.

What is left here is the CI work and the installation fixes; SPI.c and
TupleTable.c are byte-identical to whpg.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants