Repository navigation
Conversation
serprex
requested review from
JoshDreamland and
amogiska
and
a balanced review from Copilot
October 8, 2026 18:10
serprex
marked this pull request as draft
October 8, 2026 18:10
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The behavioral fix lacks regression coverage, and related usage and reference documentation remains outdated.
4 open findings
What changed in this PR
Adds PostgreSQL 19 development support and fixes cumulative row reporting for batched cursor execution.
Changes:
- Adds PostgreSQL 19 mise build support.
- Uses
es_total_processedfor cumulative row counts. - Documents PostgreSQL 19 compatibility.
| File | Description |
|---|---|
src/hooks/hooks.c |
Reports cumulative processed rows. |
scripts/run-tests.sh |
Lists PostgreSQL 19 support. |
mise.toml |
Adds PostgreSQL 19 tooling and build task. |
CLAUDE.md |
Documents PostgreSQL 19 build and API differences. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+123
to
+125
| [tasks."build:19"] | ||
| run = "rm -rf build && cmake --preset default -DPG_CONFIG=$(mise where postgres@19)/bin/pg_config && cmake --build build" | ||
| description = "Build for PostgreSQL 19" |
| usage() { | ||
| echo "Usage: $0 <PG_VERSION|PG_PATH> [test_type] [test_filter]" | ||
| echo " PG_VERSION: PostgreSQL version (16, 17, 18) - uses mise" | ||
| echo " PG_VERSION: PostgreSQL version (16, 17, 18, 19) - uses mise" |
Comment on lines
+387
to
+390
| // es_processed only counts the last ExecutorRun call; cursors/portals that | ||
| // fetch in batches run multiple times. es_total_processed accumulates across | ||
| // all runs (PG14+, same counter pg_stat_statements switched to). | ||
| event->rows = query_desc->estate->es_total_processed; |
serprex
marked this pull request as ready for review
October 8, 2026 20:21
also use es_total_processed over es_processed
| event->queryid = query_desc->plannedstmt->queryId; | ||
| event->rows = query_desc->estate->es_processed; | ||
| // Match pg_stat_statements row counts across cursor fetches. | ||
| event->rows = query_desc->estate->es_total_processed; |
Member
Author
There was a problem hiding this comment.
you say that but it builds?
Comment on lines
+387
to
+388
| // Match pg_stat_statements row counts across cursor fetches. | ||
| event->rows = query_desc->estate->es_total_processed; |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
also use es_total_processed over es_processed