Repository navigation
Let an ordering index scan hand its ORDER BY value to the target list [cf-7392] #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
835e115
4831504
a2d6bed
17721b3
6e9d59a
0add4fa
819716c
ef9ef7f
f23832d
9c22dc1
457475a
e748c83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,43 @@ | ||||||
| Diagnostics: | ||||||
| MissingIncludes: None | ||||||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (and the sibling
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (along with Critically, the fork's own
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (and .envrc, .gdbinit, flake.nix, shell.nix, pg-aliases.sh) is personal developer scaffolding unrelated to the HOT-indexed-updates change. None of it is referenced by any build/test machinery (no meson.build / Makefile mentions). PostgreSQL's minimal-diff discipline rejects unrelated files in a patch, and the repo's own root .gitignore (lines 1-3) explicitly states that local-workflow/editor files should be ignored via
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (and the sibling .envrc, .gdbinit, pg-aliases.sh, flake.nix, shell.nix, flake.lock) is personal developer-environment scaffolding that does not belong in a patch destined for pgsql-hackers. The repo's own top-level .gitignore header states that "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude" -- not committed. Committing these guarantees rejection on the list as a non-minimal diff and creates needless merge conflicts. Keep them in a local gitignore / untracked worktree instead. This is the dominant issue for the whole change.
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Developer-local editor configuration. It pins personal include paths (
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high confidence] This file should not be committed. Additionally,
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (along with .envrc, .gdbinit, pg-aliases.sh, flake.nix, shell.nix) is personal/local developer tooling that must not be committed to a PostgreSQL patch. The pgsql-hackers process requires a minimal diff containing only the code required for the stated change. I confirmed the repo does NOT ignore these artifacts (no .gitignore entry, and the .local-gitignore referenced by shell.nix does not exist), so they were committed by mistake. Move them to personal dotfiles / a local (gitignored) excludes file instead of the patch.
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (and the sibling .envrc, .gdbinit, pg-aliases.sh, flake.nix, shell.nix) is local developer-environment scaffolding that must not be committed to a PostgreSQL tree destined for pgsql-hackers/commitfest. The repo's own .gitignore states: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." Committing these violates the minimal-diff discipline, pollutes the tree, and causes needless merge conflicts. Move them to a personal git-ignored location. (high confidence)
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is personal developer-environment tooling, not part of the HOT-indexed-updates feature. For a patch destined for pgsql-hackers/commitfest it is unrelated churn that violates the minimal-diff rule and does not belong at the root of the PostgreSQL tree. Confirmed: no meson.build/Makefile references any of these files, and the repo already instructs (
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire change set (.clangd, .envrc, .gdbinit, pg-aliases.sh, flake.nix, shell.nix) is personal developer-environment scaffolding, not PostgreSQL source. For a patch destined for pgsql-hackers/commitfest this violates the "minimal diff / one logical change" discipline and will cause needless merge conflicts for everyone. The project's own root .gitignore explicitly says local tooling "should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude" -- these files belong there, not committed to the tree.
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (along with
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is personal editor/LSP scaffolding unrelated to the HOT-indexed-updates feature. It does not belong in a PostgreSQL patch destined for pgsql-hackers -- it is author-local tooling that will be rejected as non-minimal, unrelated churn. It also hardcodes author-specific assumptions (e.g. [high confidence]
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (and the sibling |
||||||
| InlayHints: | ||||||
| Enabled: true | ||||||
| ParameterNames: true | ||||||
| DeducedTypes: true | ||||||
| CompileFlags: | ||||||
| CompilationDatabase: build/ # Search build/ directory for compile_commands.json | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a developer-local workflow file, not PostgreSQL feature code. The root .gitignore explicitly documents the policy: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." A patch destined for pgsql-hackers/a commitfest must not commit per-author tooling. Additionally the include paths Confidence: high.
Comment on lines
+7
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These include hacks ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoding
Comment on lines
+7
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personal editor configuration that should not be committed to a PostgreSQL tree. It hardcodes a local layout ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The include path assumes a specific out-of-tree build layout (../../../../src/include relative to a nested build dir). Combined with |
||||||
| Remove: [ -Werror ] | ||||||
| Add: | ||||||
| - -DDEBUG | ||||||
| - -DLOCAL | ||||||
| - -DPGDLLIMPORT= | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out-of-scope developer artifact. This |
||||||
| - -DPIC | ||||||
|
Comment on lines
+13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| - -O2 | ||||||
| - -Wall | ||||||
| - -Wcast-function-type | ||||||
| - -Wconversion | ||||||
| - -Wdeclaration-after-statement | ||||||
| - -Wendif-labels | ||||||
| - -Werror=vla | ||||||
| - -Wextra | ||||||
| - -Wfloat-equal | ||||||
| - -Wformat-security | ||||||
| - -Wimplicit-fallthrough=3 | ||||||
| - -Wmissing-format-attribute | ||||||
| - -Wmissing-prototypes | ||||||
| - -Wno-format-truncation | ||||||
| - -Wno-sign-conversion | ||||||
| - -Wno-stringop-truncation | ||||||
| - -Wno-unused-const-variable | ||||||
| - -Wpointer-arith | ||||||
| - -Wshadow | ||||||
| - -Wshadow=compatible-local | ||||||
| - -fPIC | ||||||
| - -fexcess-precision=standard | ||||||
| - -fno-strict-aliasing | ||||||
| - -fvisibility=hidden | ||||||
| - -fwrapv | ||||||
| - -g | ||||||
| - -std=c11 | ||||||
| - -I. | ||||||
| - -I../../../../src/include | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The include path There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The include path
Comment on lines
+42
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The include path There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The include path
Suggested change
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||||||
| # Inherit shared CFLAGS from ~/ws/postgres/.envrc, load this worktree's | ||||||||||
| # flake, then layer the postgres agent context (helpers need flake's PATH + | ||||||||||
| # write into this worktree's dir). | ||||||||||
| source_up | ||||||||||
|
Comment on lines
+1
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file (and the sibling .clangd/.gdbinit/flake.nix/shell.nix/pg-aliases.sh) is personal developer scaffolding that must not be committed to the PostgreSQL tree. The root .gitignore explicitly states local-workflow files should be excluded via $GIT_DIR/info/exclude or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This direnv config leaks the author's local workflow: a hardcoded |
||||||||||
| watch_file flake.nix | ||||||||||
| use flake | ||||||||||
|
Comment on lines
+4
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Developer-local direnv file referencing personal paths ( Confidence: high.
Comment on lines
+4
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personal developer scaffolding that should not be committed to a PostgreSQL tree / posted to -hackers. It hardcodes the author's local workflow:
Comment on lines
+4
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This direnv file is author-local environment scaffolding and must not be committed to the tree. Beyond being unrelated to the feature, it leaks an author-specific absolute path ( [high confidence] |
||||||||||
| pg_cflags | ||||||||||
|
Comment on lines
+4
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||
| use project_steering postgresql aws c | ||||||||||
| use git_policy allow-force-push | ||||||||||
| use project_mcp postgresq | ||||||||||
|
Comment on lines
+4
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line 10 has a typo: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These direnv helpers (source_up, pg_cflags,
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This direnv file sources machine-specific, author-local helpers (
Comment on lines
+4
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high confidence] Non-portable personal config. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These direnv helpers (source_up, use project_steering, use git_policy allow-force-push, use project_mcp postgresq) depend on external, personal direnv library functions that are defined nowhere in the repo, so .envrc fails evaluation on any machine lacking them. Additionally,
Comment on lines
+4
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file depends on the author's personal There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likely typo in the direnv extension name:
Comment on lines
+4
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This .envrc is unusable outside the author's machine: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likely typo:
Comment on lines
+4
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file is non-reproducible, machine-specific personal scaffolding and must not be committed.
Comment on lines
+4
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This direnv file depends on machine-specific helpers that do not exist for anyone else: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparent typo:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out-of-scope developer artifact. This |
||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,156 @@ | ||||||
| # HOT Indexed Updates — GDB breakpoints for code review | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file is developer-local debug scaffolding for a specific in-progress patch series ("HOT Indexed Updates"). It references files and line numbers from code that is not part of this repository's committed tree (heapam_indexscan.c, heap_hot_indexed_* functions, XLOG_HEAP2_INDEXED_UPDATE), so the symbol/line breakpoints will not resolve against a build of this tree. A PostgreSQL contribution should not carry per-developer .gdbinit files; this belongs outside the repository (e.g. a local, git-ignored file). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ASCII-only is required in PostgreSQL source/diffs, but this header uses a non-ASCII em-dash (U+2014). Replace with a plain hyphen/minus if this file is kept at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These breakpoints reference code that does not exist in this change set. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-ASCII em-dash (U+2014) characters appear throughout this file's comments (section headers etc.). PostgreSQL requires ASCII-only in source and committed files. Replace the em-dashes with ASCII hyphens/dashes.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out-of-scope developer artifact. This |
||||||
| # | ||||||
| # Usage: gdb -x .gdbinit <postgres-binary> | ||||||
| # Or from gdb: source .gdbinit | ||||||
| # | ||||||
| # These breakpoints cover the major code paths introduced or modified by | ||||||
| # the HOT indexed updates patch series. They are organized by subsystem | ||||||
| # to make it easy to enable/disable groups during debugging. | ||||||
| # | ||||||
| # Tip: to skip to a specific subsystem, disable all then enable selectively: | ||||||
| # disable breakpoints | ||||||
| # enable 1 2 3 # just the update-decision group | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 1. UPDATE DECISION — heap_update() HOT/HOT-indexed/non-HOT choice | ||||||
| # src/backend/access/heap/heapam.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # Main entry: heap_update | ||||||
| break heapam.c:3210 | ||||||
|
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Breakpoints pinned to absolute source line numbers (heapam.c:3210, 4019, 4024, 4033, 4101, 4147; heapam_indexscan.c:182, 250, 297; indexam.c:299; execIndexing.c:370; pruneheap.c:1287, 1802, 1836, 1863, 2936) silently drift the moment any of those files change. I verified heapam_indexscan.c:182 in this tree is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These absolute source-line breakpoints (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolute line-number breakpoints are extremely fragile and already wrong. Verified
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line-numbered breakpoints are already wrong and are a committed personal-workflow file that does not belong in a pgsql-hackers patch (root .gitignore documents using $GIT_DIR/info/exclude for such files). Verified against the current tree: Furthermore, the symbols this file documents do not exist anywhere in the tree: Confidence: high.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These absolute line-number breakpoints (heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299, etc.) are inherently stale-prone. heapam.c is not even part of this change series, and any edit above a cited line silently shifts the breakpoint to unrelated code, giving a false sense of debugging coverage. Line-number breakpoints are unmaintainable in a tree under active development; prefer function-name breakpoints (as already done elsewhere in this file, e.g.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file is developer-local scaffolding and must not be committed to the tree. It hardcodes absolute line-number breakpoints (e.g.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line-number breakpoints are fragile and already wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high confidence] Line-number breakpoints are fragile and already partly stale. Breakpoints pinned to absolute source lines (
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These breakpoints reference symbols (heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, heap_hot_search_buffer chain helpers, etc.) and absolute line numbers (heapam.c:3210/4019/4024/..., pruneheap.c:1802/2936, indexam.c:299) that do not exist in this tree. I verified the heap_hot_indexed_* symbols appear nowhere except this file, so these breakpoints document an unmerged patch series that is not part of this change. Hardcoded source line numbers are also extremely fragile: any edit to the referenced files silently invalidates every numeric breakpoint. This is debug scaffolding, not reviewable product code; it should not be committed.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded source line-number breakpoints are already stale and will silently drift on any edit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These breakpoints reference functions that do not exist anywhere in the current tree (heap_hot_indexed_create_tuple, heap_hot_indexed_tuple_size, heap_hot_indexed_serialize_bitmap, heap_xlog_indexed_update, etc. all resolve to zero symbols). They belong to an unlanded HOT-indexed-update patch series that is not part of this change. The hardcoded absolute line-number breakpoints (heapam.c:3210, :4019, :4024, :4033, heapam_indexscan.c:182, :250, :297, pruneheap.c:1802, indexam.c:299, execIndexing.c:370) silently point at the wrong code whenever any line shifts — a footgun even for a local file. This file should not be committed. (high confidence)
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Breakpoints are encoded as hardcoded source line numbers (e.g.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is personal developer scaffolding that must not be part of a patch posted to pgsql-hackers. Beyond being non-minimal and unrelated to a committable change, it is actively broken/misleading:
This belongs in the developer's local-only ignore (shell.nix already sets
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These absolute source line-number breakpoints are tied to an unrelated "HOT indexed updates" patch series and are already wrong against this tree. For example, pruneheap.c:1802 is not "collect bitmaps from dead INDEXED_UPDATED intermediates" -- in this tree line 1802 is
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These line-number breakpoints are fragile and already stale.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These breakpoints reference symbols and line numbers that do not exist in the tree. None of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded source line-number breakpoints (e.g.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a review-only, author-local GDB breakpoint list that should not be committed. Two concrete defects compound the scope problem:
[high confidence] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These line-number breakpoints target files that the feature itself modifies (heapam.c, pruneheap.c, indexam.c, execIndexing.c). Line numbers drift the instant those files change, so this file is stale on arrival. Worse, it references symbols that do not exist anywhere in the tree (verified: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These line-number breakpoints are already wrong against this tree. |
||||||
|
|
||||||
| # HOT decision block: pure HOT vs HOT indexed vs non-HOT | ||||||
| # Line 4019: pure HOT (no indexed columns changed) | ||||||
| # Line 4024: HOT indexed path (non-catalog, some indexed columns changed) | ||||||
| # Line 4031: predict augmented tuple size | ||||||
| # Line 4033: size+space check before creating augmented tuple | ||||||
| break heapam.c:4019 | ||||||
| break heapam.c:4024 | ||||||
| break heapam.c:4033 | ||||||
|
|
||||||
| # Set HEAP_INDEXED_UPDATED flag on new tuple before page insertion | ||||||
| break heapam.c:4101 | ||||||
|
|
||||||
| # Restore HEAP_INDEXED_UPDATED on old tuple (only if it previously had it) | ||||||
| break heapam.c:4147 | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 2. TUPLE CREATION — building the augmented tuple with embedded bitmap | ||||||
| # src/backend/access/heap/heapam.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # Predict augmented tuple size (returns 0 if t_hoff would overflow) | ||||||
| break heap_hot_indexed_tuple_size | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These function-name breakpoints reference symbols that do not exist anywhere in this tree (verified: no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These symbols do not exist anywhere in the tree. A codebase search finds no
Comment on lines
+42
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These function-name breakpoints reference symbols that do not exist anywhere in this tree (heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, heap_hot_indexed_bitmap_* , heap_hot_indexed_read_bitmap, heap_xlog_indexed_update, etc. appear only in this .gdbinit). The heap-side "HOT indexed updates" functions this file is meant to debug were never added by this change set, so GDB will fail to set these breakpoints. This file does not belong in the patch and is non-functional against the actual source. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These symbol-based breakpoints reference HOT-indexed helper functions ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These breakpoints target a "HOT indexed updates" patch series that is not present in this tree. I verified that none of the referenced functions exist here: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These function-name breakpoints reference symbols that do not exist anywhere in this tree ( |
||||||
|
|
||||||
| # Create augmented tuple with embedded modified-column bitmap | ||||||
| break heap_hot_indexed_create_tuple | ||||||
|
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These breakpoints target a "HOT indexed updates" patch series that is not present in this tree. Verified that
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These symbolic breakpoints target functions/records that do not exist anywhere in the tree:
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These symbols (heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, heap_hot_search_buffer, etc.) and the HEAP2_INDEXED_UPDATE WAL path do not exist in this tree -- a code search for heap_hot_indexed* / HEAP_INDEXED_UPDATED returns no matches in src/. The breakpoints reference a different branch's code, so this file is unusable here and confirms it is foreign scaffolding. |
||||||
|
|
||||||
| # Serialize Bitmapset into raw bytes in tuple header | ||||||
| break heap_hot_indexed_serialize_bitmap | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 3. BITMAP UTILITIES — raw bitmap operations for chain following | ||||||
| # src/backend/access/heap/heapam.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # Compute raw bitmap byte size from natts | ||||||
| break heap_hot_indexed_bitmap_raw_size | ||||||
|
|
||||||
| # Check if tuple header has room for bitmap between null bitmap and data | ||||||
| break heap_hot_indexed_has_bitmap_space | ||||||
|
|
||||||
| # Read HOT indexed bitmap from tuple header (returns Bitmapset) | ||||||
| break heap_hot_indexed_read_bitmap | ||||||
|
|
||||||
| # Fast overlap check: does tuple's raw bitmap overlap with indexed_attrs? | ||||||
| break heap_hot_indexed_bitmap_overlaps_raw | ||||||
|
|
||||||
| # OR a tuple's raw bitmap into an accumulator buffer | ||||||
| break heap_hot_indexed_bitmap_or_raw | ||||||
|
|
||||||
| # Check if accumulated raw bitmap overlaps with indexed_attrs | ||||||
| break heap_hot_indexed_accum_overlaps | ||||||
|
|
||||||
| # Merge bitmaps from dead tuples into a target tuple on the page | ||||||
| break heap_hot_indexed_merge_bitmaps_raw | ||||||
|
|
||||||
| # Deserialize raw bytes back to Bitmapset | ||||||
| break heap_hot_indexed_deserialize_bitmap | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 4. INDEX SCAN — HOT chain following with stale-entry detection | ||||||
| # src/backend/access/heap/heapam_indexscan.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # Main HOT chain search with indexed update awareness | ||||||
| break heap_hot_search_buffer | ||||||
|
|
||||||
| # Redirect-with-data: initialize bitmap accumulator from collapsed redirect | ||||||
| break heapam_indexscan.c:182 | ||||||
|
Comment on lines
+88
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolute source line-number breakpoints (heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299, etc.) are extremely fragile: any edit to these files silently shifts the breakpoint to unrelated code. None of these files are in this changeset, so the line numbers are unverified and almost certainly already stale. If such a helper is kept at all (it should not be committed), it must use function-name/symbol breakpoints, never raw line numbers. |
||||||
|
|
||||||
| # Accumulate bitmap from INDEXED_UPDATED tuple in chain | ||||||
| break heapam_indexscan.c:250 | ||||||
|
|
||||||
| # Stale entry detection: accumulated bitmap overlaps this index's attrs | ||||||
| break heapam_indexscan.c:297 | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 5. INDEX SCAN SETUP — indexed_attrs bitmap computation | ||||||
| # src/backend/access/index/indexam.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # Compute indexed_attrs for HOT indexed update chain following | ||||||
| break indexam.c:299 | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 6. INDEX INSERTION — skip unchanged indexes for HOT indexed updates | ||||||
| # src/backend/executor/execIndexing.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # Entry: insert/update index tuples | ||||||
| break ExecInsertIndexTuples | ||||||
|
|
||||||
| # Index skip decision: skip indexes whose attrs don't overlap modified set | ||||||
| break execIndexing.c:370 | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 7. PRUNING — chain collapsing and redirect-with-data | ||||||
| # src/backend/access/heap/pruneheap.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # Main prune function | ||||||
| break heap_page_prune_and_freeze | ||||||
|
|
||||||
| # Per-chain pruning entry | ||||||
| break heap_prune_chain | ||||||
|
|
||||||
| # Chain collapsing: collect bitmaps from dead INDEXED_UPDATED intermediates | ||||||
| break pruneheap.c:1802 | ||||||
|
|
||||||
| # OR dead tuple bitmaps into combined bitmap | ||||||
| break pruneheap.c:1836 | ||||||
|
|
||||||
| # Record redirect-with-data for execute phase | ||||||
| break pruneheap.c:1863 | ||||||
|
|
||||||
| # Execute phase: apply redirect-with-data entries on the page | ||||||
| break pruneheap.c:1287 | ||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 8. WAL REPLAY — recovery of HOT indexed updates | ||||||
| # src/backend/access/heap/heapam_xlog.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # WAL replay for XLOG_HEAP2_INDEXED_UPDATE | ||||||
| break heap_xlog_indexed_update | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
|
|
||||||
| # ========================================================================= | ||||||
| # 9. WAL LOGGING — writing HOT indexed update records | ||||||
| # src/backend/access/heap/heapam.c | ||||||
| # ========================================================================= | ||||||
|
|
||||||
| # WAL logging for heap updates (handles indexed_update flag) | ||||||
| break log_heap_update | ||||||
|
|
||||||
| # Serialize redirect-with-data into WAL record (pruneheap.c) | ||||||
| break pruneheap.c:2936 | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Weekly check for a newer Claude Opus on Bedrock; opens/updates one issue here | ||
| # when OCR_BEDROCK_MODEL is behind. Notify-only (GITHUB_TOKEN cannot write | ||
| # Actions variables). Logic lives in gburd/ci-workflows. | ||
|
|
||
| name: OCR model self-check | ||
|
|
||
| on: | ||
| schedule: | ||
| - cron: '0 12 * * 1' # Mondays 12:00 UTC | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| check: | ||
| uses: gburd/ci-workflows/.github/workflows/ocr-model-check.yml@v1 | ||
| with: | ||
| aws_role_arn: ${{ vars.AWS_ROLE_ARN }} | ||
| aws_region: ${{ vars.AWS_REGION }} | ||
| bedrock_model: ${{ vars.OCR_BEDROCK_MODEL }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # AI PR review (Claude Opus on Bedrock). Logic and review rules live in | ||
| # gburd/ci-workflows so this fork's master carries a stub instead of ~1000 | ||
| # lines that have to be rebased onto upstream forever. | ||
| # | ||
| # Needs repo variables AWS_ROLE_ARN, AWS_REGION, OCR_BEDROCK_MODEL. Auth is | ||
| # GitHub OIDC; no static AWS keys. See the ci-workflows README. | ||
|
|
||
| name: OCR AI Review | ||
|
|
||
| on: | ||
| pull_request: | ||
| # Note: no draft filter — drafts are reviewed too. | ||
| types: [opened, synchronize, reopened, ready_for_review] | ||
| issue_comment: | ||
| types: [created] | ||
| workflow_dispatch: | ||
| inputs: | ||
| pr_number: | ||
| description: 'PR number to review' | ||
| required: true | ||
| type: number | ||
|
|
||
| # One review per PR; cancel superseded runs to save Bedrock spend. | ||
| concurrency: | ||
| group: ocr-review-${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| review: | ||
| # Comment events only when the comment is on a PR and starts with the | ||
| # trigger keyword; PR events and manual dispatch always. | ||
| if: | | ||
| github.event_name == 'pull_request' || | ||
| github.event_name == 'workflow_dispatch' || | ||
| (github.event_name == 'issue_comment' && github.event.issue.pull_request && | ||
| (startsWith(github.event.comment.body, '/open-code-review') || | ||
| startsWith(github.event.comment.body, '@open-code-review') || | ||
| startsWith(github.event.comment.body, '/pg-history'))) | ||
| uses: gburd/ci-workflows/.github/workflows/ocr-review.yml@v1 | ||
| with: | ||
| aws_role_arn: ${{ vars.AWS_ROLE_ARN }} | ||
| aws_region: ${{ vars.AWS_REGION }} | ||
| bedrock_model: ${{ vars.OCR_BEDROCK_MODEL }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Keep this fork's master rebased on postgres/postgres master, carrying only | ||
| # the fork-owned CI stubs (.github/workflows/fork-*.yml). The workflows they | ||
| # call, and the OCR review rules, live in gburd/ci-workflows. | ||
| # | ||
| # Requires the SYNC_TOKEN secret: a PAT with repo+workflow scope. The default | ||
| # GITHUB_TOKEN is not allowed to push commits that touch .github/workflows/, | ||
| # and every commit we carry does. | ||
|
|
||
| name: Sync from upstream | ||
|
|
||
| on: | ||
| schedule: | ||
| - cron: '17 */3 * * *' | ||
| workflow_dispatch: | ||
|
|
||
| concurrency: | ||
| group: fork-sync-upstream | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| sync: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 0 | ||
| token: ${{ secrets.SYNC_TOKEN }} | ||
|
|
||
| - name: Rebase master onto upstream | ||
| run: | | ||
| set -euo pipefail | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| git remote add upstream https://github.com/postgres/postgres.git | ||
| git fetch --no-tags upstream master | ||
|
|
||
| # Master carries fork CI only. Anything else means work landed here | ||
| # that belongs on a branch; force-rebasing it on a timer would be wrong. | ||
| if git diff --name-only upstream/master...HEAD | | ||
| grep -v '^\.github/workflows/fork-'; then | ||
| echo "::error::master carries non-CI changes (listed above); move them to a branch" | ||
| exit 1 | ||
| fi | ||
|
|
||
| git rebase upstream/master | ||
| git push --force-with-lease |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,3 +35,21 @@ SELECT count(*) FROM timestamptmp WHERE a > '2004-10-26 08:55:08'::timestamp; | |
| EXPLAIN (COSTS OFF) | ||
| SELECT a, a <-> '2004-10-26 08:55:08' FROM timestamptmp ORDER BY a <-> '2004-10-26 08:55:08' LIMIT 3; | ||
| SELECT a, a <-> '2004-10-26 08:55:08' FROM timestamptmp ORDER BY a <-> '2004-10-26 08:55:08' LIMIT 3; | ||
|
|
||
| -- An ordering scan that hands an exact point distance to the target list | ||
| -- while another ORDER BY key, here an interval, is of a by-reference type. | ||
| CREATE TABLE timestamppt (p point, a timestamp); | ||
| INSERT INTO timestamppt | ||
| SELECT point(i % 7, i % 11), '2004-10-26 08:55:08'::timestamp + i * interval '37 min' | ||
| FROM generate_series(1, 600) i; | ||
| CREATE INDEX timestamppt_idx ON timestamppt USING gist (p, a); | ||
| SET enable_indexonlyscan=off; | ||
| EXPLAIN (VERBOSE, COSTS OFF) | ||
| SELECT p <-> point(3, 4) AS d, a <-> '2004-10-26 08:55:08' AS da | ||
| FROM timestamppt | ||
| ORDER BY p <-> point(3, 4), a <-> '2004-10-26 08:55:08' LIMIT 3; | ||
| SELECT p <-> point(3, 4) AS d, a <-> '2004-10-26 08:55:08' AS da | ||
|
Comment on lines
+50
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The final query orders only by |
||
| FROM timestamppt | ||
| ORDER BY p <-> point(3, 4), a <-> '2004-10-26 08:55:08' LIMIT 3; | ||
| RESET enable_indexonlyscan; | ||
| DROP TABLE timestamppt; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file (along with .envrc, .gdbinit, pg-aliases.sh, flake.nix, shell.nix) is personal developer-environment tooling and does not belong in a PostgreSQL contribution destined for pgsql-hackers/commitfest. It is unrelated to the backend change (the HOT indexed-updates series reflected in the other modified files) and violates the minimal-diff discipline; a reviewer will reject the patch for bundling local scaffolding. The repo's own root .gitignore states that such files 'should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude' -- that is where this belongs, not committed to the tree.