Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds PostgreSQL 19 support across CI workflows, documentation, Docker build logic, compatibility shims, and version-gated C code paths. ChangesPostgreSQL 19 support
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/getting_started.md`:
- Line 78: The PostgreSQL version substitution guidance is too vague because the
examples still hard-code 18 in multiple places. Update the getting started text
to explicitly say users must replace all version-specific references together,
and point readers to the relevant example identifiers such as the package names,
paths, and the postgresql-18 service name so they know every occurrence must be
changed consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68bdea0f-59ee-456f-96f7-0568d18d7535
📒 Files selected for processing (17)
.github/workflows/installcheck.yml.github/workflows/nightly_tap.yml.github/workflows/pg-stable-test.yml.github/workflows/random-delays-regress.yml.github/workflows/spockbench.yml.github/workflows/zodan_sync.ymlREADME.mddocs/getting_started.mddocs/index.mddocs/spock_release_notes.mdsrc/spock_conflict_stat.csrc/spock_functions.csrc/spock_output.csrc/spock_progress_recovery.csrc/spock_proto_native.csrc/spock_readonly.ctests/docker/Dockerfile-step-1.el9
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/spock_proto_native.c (1)
1028-1028: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType change is functionally a no-op; prefer
bits16for API-alignment clarity.
bits16is a typedef ofuint16, so this compiles fine either way, but core'sformat_type_extended()declares itsflagsparameter asbits16. Keeping the local variable typed asbits16better documents intent and matches the core API it's passed into.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/spock_proto_native.c` at line 1028, The local flags variable in the formatting path is using uint16 even though it is passed into format_type_extended(), whose flags parameter is bits16. Update the declaration in this area to use bits16 instead, keeping the existing value and behavior unchanged, so the type matches the core API and the intent is clearer.src/spock_proto_json.c (1)
221-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider documenting the rename rationale.
Unlike the other PG19 shims in this PR (spock.c, spock_output_plugin.h, spock_jsonb_utils.h), this rename has no comment explaining why. Core PostgreSQL exposed a global
composite_to_json()(exported forCOPY TO ... FORMAT json), which would otherwise clash with this file's static function on PG19+. A short comment would prevent a future accidental revert.📝 Suggested comment
-static void spock_composite_to_json(Datum composite, StringInfo result, +/* Renamed from composite_to_json to avoid clashing with the symbol + * PostgreSQL core exports for COPY TO ... FORMAT json support. */ +static void spock_composite_to_json(Datum composite, StringInfo result, bool use_line_feeds);Also applies to: 569-569
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/spock_proto_json.c` at line 221, Add a brief explanatory comment near spock_composite_to_json stating that the rename is intentional on PG19+ to avoid clashing with PostgreSQL’s exported global composite_to_json() used by COPY TO ... FORMAT json. Update the surrounding shim comment in this area so future readers understand why this static name differs from the other PG19 compatibility shims and won’t accidentally revert it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/spock_proto_json.c`:
- Line 221: Add a brief explanatory comment near spock_composite_to_json stating
that the rename is intentional on PG19+ to avoid clashing with PostgreSQL’s
exported global composite_to_json() used by COPY TO ... FORMAT json. Update the
surrounding shim comment in this area so future readers understand why this
static name differs from the other PG19 compatibility shims and won’t
accidentally revert it.
In `@src/spock_proto_native.c`:
- Line 1028: The local flags variable in the formatting path is using uint16
even though it is passed into format_type_extended(), whose flags parameter is
bits16. Update the declaration in this area to use bits16 instead, keeping the
existing value and behavior unchanged, so the type matches the core API and the
intent is clearer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc5647ec-e431-4d98-955a-3370fc043380
📒 Files selected for processing (9)
include/spock_jsonb_utils.hinclude/spock_output_plugin.hsrc/compat/19/spock_compat.hsrc/spock.csrc/spock_apply.csrc/spock_output_plugin.csrc/spock_proto_json.csrc/spock_proto_native.csrc/spock_relcache.c
✅ Files skipped from review due to trivial changes (1)
- src/spock_apply.c
🚧 Files skipped from review as they are similar to previous changes (1)
- src/compat/19/spock_compat.h
danolivo
force-pushed
the
spoc-610
branch
3 times, most recently
from
fb72116 to
391be09
Compare
Version shims for PG19: RepOriginId->ReplOriginId, the replorigin session globals folded into replorigin_xact_state, index_beginscan flags argument, ShmemInitHash losing max_size, AssertVariableIsOfType->StaticAssertVariableIsOfType, and isQueryUsingTempRelation->query_uses_temp_object. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Guard or redirect the call sites affected by PG19: the exposed server_message_level_options/composite_to_json, the flattened ReorderBufferTXN xact_time, JsonbInState, RepackStmt (was ClusterStmt), SignalRecoveryConflictWithVirtualXID, pgstat_fetch_entry/get_database_name relocations, and the wait_event.h include dropped from pgstat.h. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add pg19 to the workflow matrices. PG19 has no GA tag yet, so the Docker build temporarily tracks the tip of REL_19_STABLE instead of a tag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
List 19 as supported, note the version-substitution caveat in getting started, and add a 6.0.0 release-note highlight. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
No description provided.