Skip to content

CI: fix PG 19 RC1 build and lolor TAP tests - #653

Merged
mason-sharp merged 4 commits into
mainfrom
lolor-latest-release-tag
Oct 17, 2026
Merged

mason-sharp merged 4 commits into
mainfrom
lolor-latest-release-tag

Conversation

@ibrarahmad

@ibrarahmad ibrarahmad commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Supersedes #651, whose two commits are included here with one comment re-wrapped to satisfy pgindent, plus one further change to how the lolor TAP tests obtain lolor.

PG 19 RC1 picked up postgres/postgres@2f7bf51b7b, which added a skipduplicates argument to RelationFindReplTupleByIndex() and turned the -Werror, regression and random-delays workflows red. FindReplTupleInLocalRel() always passes the replica identity index from the relation map, so it passes true. Back branches keep the old signature and are untouched.

ensure_lolor() in tests/tap/t/SpockTest.pm cloned lolor's main branch, so an unreleased commit there could fail tests 032 and 033 for reasons that have nothing to do with spock. It now clones the newest release tag. lolor publishes no GitHub releases — GET /repos/pgEdge/lolor/releases returns an empty list — so the release is the highest vN.N.N tag, currently v1.2.3. Pre-release tags such as v1.2.3-rc.1 carry a suffix and are excluded by the pattern, and sort -V orders the rest by version rather than lexically. If no tag resolves, ensure_lolor() returns false and the caller skips, rather than silently falling back to main.

Clusters created after a successful ensure_lolor() also add lolor to shared_preload_libraries. v1.2.3 does not require that, but lolor main does, so the tests keep working once the next release is tagged.

This pin takes 033_zodan_lolor_add_node from 19 failing subtests to 9: the DROP EXTENSION lolor failures reported in #651 go away, because pgEdge/lolor#62 landed after v1.2.3. The nine that remain are the large object migration assertions, which no released lolor can satisfy, so the last commit runs that round-trip only where lolor carries migrate_from_native() and expects just the newly created object otherwise. The drop, the recreate and the replication set checks around it run either way. Verified on PG 18: 79/79 against lolor 1.3.0 where the migration runs, and 79/79 against v1.2.3 where those six are skipped.

ensure_lolor() cloned lolor's main branch, so an unreleased commit there
could break the lolor TAP tests for reasons unrelated to spock.

lolor publishes no GitHub releases, only tags, so resolve the highest
vN.N.N tag and clone that.  Pre-release tags are excluded by the
pattern, and a tag that cannot be resolved makes the caller skip rather
than fall back to main.
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3927d976-3ca1-498a-af02-6cc1baa52fab
📥 Commits

Reviewing files that changed from the base of the PR and between 1fb2efd and a8363fa.

📒 Files selected for processing: 1
  • tests/tap/t/033_zodan_lolor_add_node.pl

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The changes update lolor selection, installation checks, and preload configuration in TAP tests. They also adjust lolor migration assertions based on function availability. Tuple lookup uses the PostgreSQL 19 signature while retaining the earlier signature for previous PostgreSQL versions.

Changes: lolor test setup

Layer / File(s) Summary
Select and clone a stable tag
tests/tap/t/SpockTest.pm
ensure_lolor records lolor availability and selects the highest matching vN[.N...] tag. It returns false if no tag matches, cloning or installation fails, or the installed control file is absent.
Configure lolor preloading
tests/tap/t/SpockTest.pm
create_postgresql_conf includes lolor alongside spock when lolor is available. Otherwise, it includes only spock.
Check lolor migration behavior
tests/tap/t/033_zodan_lolor_add_node.pl
The test checks for lolor.migrate_from_native(). When available, it verifies migration on three nodes and expects four lolor large objects. Otherwise, it skips six migration assertions and expects one lolor large object.

PostgreSQL 19 tuple lookup

Layer / File(s) Summary
Use the PostgreSQL 19 lookup signature
src/spock_apply_heap.c
For PostgreSQL 19 and later, FindReplTupleInLocalRel passes true as the first-match argument. Earlier PostgreSQL versions retain the previous call signature.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to a8363

The lolor tests skip when setup is unavailable, migration checks follow the installed API, and the PostgreSQL 19 lookup call matches its API contract. No actionable merge risk was established in the reviewed changes.

### Architecture Summary

Architecture risk: 🔵 Low · up to a8363

The change affects 2 systems.

Changed systems: tests, src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — tests (service) was modified; 2 changed files map to changed impact.
  • observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in tests/tap/t/SpockTest.pm: ensure_lolor() now records availability in $preload_lolor, including when lolor is already installed. Instead of cloning the default branch, it finds the highest tag matching the numeric vN[.N...] pattern and clones that tag; if no matching tag is available, cloning or installation fails, or the installed control file is absent afterward, it returns false.
  • observed — Modified behavior in tests/tap/t/SpockTest.pm: create_postgresql_conf now includes lolor alongside spock in shared_preload_libraries when $preload_lolor is true; otherwise the setting remains spock.
  • observed — Modified behavior in src/spock_apply_heap.c: For PG19 and later, FindReplTupleInLocalRel calls RelationFindReplTupleByIndex with the added true argument; the comment identifies this as allowing the first index match without comparison. Earlier PostgreSQL versions continue using the previous signature.
  • observed — Modified behavior in tests/tap/t/033_zodan_lolor_add_node.pl: The test now detects whether lolor.migrate_from_native() exists and skips six migration assertions when it does not. When present, it verifies that each node has three native large objects after the drop and migrates all three back into lolor.
Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly summarizes the two main changes: the PostgreSQL 19 build fix and the lolor TAP test fixes.
Description check Passed The description directly explains the PostgreSQL 19 API change, lolor version pinning, preload configuration, and TAP test behavior.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the tags at dawn,
Then sets the preload before moving on.
Three nodes test migration’s way,
While PostgreSQL versions call as they may.
I nibble greens and hop away.

Comment @coderabbitai help to get the list of available commands.

@codacy-production

codacy-production Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@ibrarahmad ibrarahmad self-assigned this Oct 15, 2026
@ibrarahmad ibrarahmad added the bug Something isn't working label Oct 15, 2026
@ibrarahmad ibrarahmad changed the title tests: build lolor from its latest release tag, not main CI: fix PG 19 RC1 build and lolor TAP tests Oct 16, 2026
PostgreSQL commit 2f7bf51b7b (in REL_19_RC1) added a skipduplicates
argument to RelationFindReplTupleByIndex(): the caller now says whether
the first index match may be taken without comparing it against the
search slot, instead of the function re-deriving it from the index.
Back branches keep the old signature and add
RelationFindReplTupleByIndexExt() instead, so only PG 19 is affected.

FindReplTupleInLocalRel() is only called with the replica identity
index recorded in the relation map, so pass true.

The -Werror and regression workflows build the newest REL_19_* tag and
started failing once REL_19_RC1 appeared.
ensure_lolor() builds lolor from its master branch, which now refuses
CREATE EXTENSION unless the library is in shared_preload_libraries.
032_lolor_largeobject_repset and 033_zodan_lolor_add_node therefore
failed at their first CREATE EXTENSION lolor.

Remember a successful ensure_lolor() and add lolor to
shared_preload_libraries for every cluster created after it.
lolor's migrate_from_native(), and the drop that leaves the large
objects in pg_catalog for it to pick up, arrived after v1.2.3, the
newest release ensure_lolor() now builds.  Skip that round-trip when the
installed lolor lacks the function, and expect only the newly created
large object afterwards.  The drop, the recreate and the replication set
checks around it run either way.
@mason-sharp
mason-sharp merged commit 31dbb6c into main Oct 17, 2026
39 checks passed
@mason-sharp
mason-sharp deleted the lolor-latest-release-tag branch October 17, 2026 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants