Repository navigation
Cooling-stage clock sweep (batched sweep + HOT/COOL evictor + strategy removal) #34
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
b63f7d1
0e63f05
cf4f0c6
4d2fa4f
5ad4f24
ab69ff6
9634225
4fbc4b4
6ebbba6
aab9afd
ec279e0
1f6a1db
9a8ab30
4782aa5
16cd1cd
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 should not be committed to the repository. The project's root Additionally, note the paths are developer-specific:
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
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 file should not be committed. It is a personal editor/LSP configuration, not part of PostgreSQL's build or source. The project's own top-level .gitignore documents this policy explicitly: "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/commitfest must be a minimal diff; unrelated tooling/config files like this are a top rejection reason. Please drop this file from the patch and ignore it locally instead. (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
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 Confidence: high.
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 file should not be part of the patch. PostgreSQL's own top-level
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
If local clangd config is desired, it belongs in the developer's own environment (or
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 .clangd is personal editor scaffolding, not part of the HOT-indexed-updates feature. The top-level .gitignore explicitly directs auxiliary/editor files to be ignored locally 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. [high confidence] This is personal developer-environment config and must not be committed to a patch destined for pgsql-hackers/commitfest. The repository's top-level
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. Editor-local config that should not be committed. The repository'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 entire group of files (.clangd, .gdbinit, flake.nix, shell.nix, pg-aliases.sh) is personal developer/editor/debugger/Nix tooling that is unrelated to the actual change in this update (the pg_buffercache / NUMA clock-sweep partitioning work in freelist.c, buf_init.c, bufmgr.c, etc.). PostgreSQL's own .gitignore says so explicitly: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." Committing this scaffolding violates the minimal-diff discipline, is not part of any pgsql-hackers patch, and will cause needless merge conflicts for other contributors. These files should be removed from the patch and kept out of tree via a local/global gitignore instead.
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 is personal editor/clangd tooling and does not belong in a PostgreSQL contribution. Per upstream patch-hygiene, a patch must be a minimal diff containing only what the stated feature (HOT indexed updates) requires. Committing a clangd config to the repo root pollutes the tree, has no tests or docs, and would be rejected on -hackers. There is no precedent in the tree's .gitignore files for committing such per-developer config. Keep it out of the tree (local, untracked) or in a personal dotfiles setup.
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 .gdbinit, pg-aliases.sh, flake.nix, shell.nix) is developer-local tooling and should not be committed to the PostgreSQL tree. The repository's own root .gitignore states the policy explicitly: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." Committing editor/dev-environment config pollutes the tree, creates merge conflicts, and violates the minimal-diff discipline for a patch destined for pgsql-hackers/commitfest. These files belong in a personal local-excludes setup, not in 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. Editor-local clangd configuration should not be committed to the PostgreSQL tree. It is not part of the logical change, pollutes the repo for other contributors, and the hardcoded relative include path
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/LSP configuration that should not be committed to the PostgreSQL tree. Per the root |
||||||
| 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. This Additionally the config is not portable to other contributors: 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 smart quote / em-dash characters appear in the added comments (this line uses
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. This is a per-developer IDE environment file ( |
||||||
| Remove: [ -Werror ] | ||||||
| Add: | ||||||
| - -DDEBUG | ||||||
| - -DLOCAL | ||||||
| - -DPGDLLIMPORT= | ||||||
| - -DPIC | ||||||
| - -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. This include path is almost certainly wrong for a
Suggested change
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
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
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. Hardcoded relative 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 relative include path Confidence: high.
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. This hardcoded relative include path is fragile. Since
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
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.
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 relative include paths
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. This is developer-local editor configuration and, together with 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 uses a fixed relative depth (../../../../src/include) that only resolves from one specific build subdirectory. It will silently fail to find headers when clangd is launched from the repo root or any other depth, defeating the purpose of the config. If this file were to stay (it should not; see the top-level comment), prefer -Isrc/include relative to the project root or rely entirely on 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.
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 fixed relative include |
||||||
| 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 a personal debugging aid and must be removed from the patch. Committing a 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 ( 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 header (and several other comment lines) use a non-ASCII em-dash character ('—'), which violates the project's ASCII-only rule for source and diffs. Use a plain ASCII hyphen/dash instead.
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. The em-dash (U+2014) violates the ASCII-only rule for source and diffs. If this file were to be kept at all, use an ASCII hyphen. (More fundamentally, the whole file should not be committed.) Confidence: high.
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. This entire 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 characters (em dash U+2014) appear in the header and inline comments, violating the ASCII-only rule for source and diffs. If any of this were retained it would fail hygiene checks; use a plain hyphen
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. This repository-root 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 source/diffs. This line (and the section headers below, e.g. lines 15, 38, 52, ...) use a Unicode em-dash (U+2014 —) rather than a plain hyphen. Replace every em-dash with
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. 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. This entire file is a developer-local debugging artifact and must not be committed. The repository's own .gitignore states at the top: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." A .gdbinit at the repo root is exactly such a file. Committing it violates the minimal-diff / one-logical-change discipline and would be rejected on pgsql-hackers as unrelated churn. Remove it from the change set and add it to your local excludes instead. 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 should not be committed. 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 character ('—') violates the ASCII-only-in-source discipline. This appears in the header line and repeatedly throughout the file (e.g., subsystem headers). Since the file should be removed entirely this is moot, but if any comparable file were kept it must use ASCII ('-' 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 entire file is a personal developer debugging artifact, not part of any feature. A patch destined for pgsql-hackers/commitfest must be minimal and contain only what the change requires. Committing a top-level 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 header uses a non-ASCII em-dash. PostgreSQL sources are ASCII-only (no smart quotes, em-dashes, or ellipsis characters). If this file were kept at all it must use a plain ASCII hyphen.
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. Non-ASCII em-dash characters (U+2014 "—") are used throughout the comments here, violating the project's ASCII-only rule for source and diffs. If this file were to be kept (it should not), these would need to be replaced with an ASCII hyphen/dash. Severity: low confidence — minor hygiene issue, secondary to the file not belonging in the patch 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. This entire file must not be committed to a PostgreSQL patch submission. It is a personal debugging artifact that violates the minimal-diff discipline (a top rejection reason on pgsql-hackers). It is also a footgun: GDB auto-loads .gdbinit from the current directory, so anyone invoking gdb from the repo root will silently execute these breakpoint commands. Worse, verification shows this is orphaned scaffolding: the referenced helper functions (heap_hot_indexed_create_tuple, heap_hot_indexed_tuple_size, heap_hot_indexed_serialize_bitmap, heap_xlog_indexed_update, the HEAP_INDEXED_UPDATED flag, etc.) do NOT exist anywhere in this tree, and none of the HOT-indexed-update code they target is part of this changeset. Remove this file (keep it local, e.g. in a personal gitignore). 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 Beyond scope, this is a security footgun: GDB auto-loads 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 personal developer scaffolding and does not belong in a patch destined for pgsql-hackers/commitfest. It references a feature that is not present in this change: a codebase search confirms 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. This file contains non-ASCII em-dash characters (U+2014
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. GDB auto-sources 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 debugging state that does not belong in a patch destined for pgsql-hackers. Beyond the minimal-diff objection, it is actively broken: the breakpoints hardcode absolute source line numbers ( 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 characters (em-dash 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 developer-only 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 characters: these comment lines use em-dashes (U+2014
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. This entire file is unrelated to the PR's subject (the NUMA/pg_buffercache work in the other changed files). It is a personal debugging helper and should not be committed to the PostgreSQL tree. Bundling unrelated local tooling into a patch destined for pgsql-hackers is a top rejection reason. Worse, these breakpoints target code that does not exist in this branch: 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. This entire file is machine-specific developer scaffolding and should not be committed to the shared tree. It targets a "HOT indexed updates" patch series whose symbols do not exist in this repository: |
||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||
| # Usage: gdb -x .gdbinit <postgres-binary> | ||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+3
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 Worse, the file is orphaned from the feature it targets: the breakpoints reference a "HOT indexed updates" implementation that is NOT present in this tree. Symbols such as
Comment on lines
+1
to
+3
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 a personal debugging artifact and must not be committed to the PostgreSQL tree. GDB auto-loads Confidence: high. |
||||||||||||||||||||||||||||||
| # Or from gdb: source .gdbinit | ||||||||||||||||||||||||||||||
|
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 is a personal developer-tooling artifact (a GDB breakpoint script) and must not be committed to the PostgreSQL tree. It is added alongside two other local scratch files in this same change (
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 Severity: high confidence. |
||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||
| # These breakpoints cover the major code paths introduced or modified by | ||||||||||||||||||||||||||||||
|
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 and diffs. This comment block uses non-ASCII em-dashes (e.g., "HOT Indexed Updates —", "returns Bitmapset" lines and the "—" separators throughout). Another reason this file cannot be committed as-is. |
||||||||||||||||||||||||||||||
| # the HOT indexed updates patch series. They are organized by subsystem | ||||||||||||||||||||||||||||||
|
Comment on lines
+6
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. Topic mismatch: this file's stated purpose is a "HOT Indexed Updates" feature, but that feature is not present in this change set (the actual modified files are buffer-manager code: bufmgr.c, freelist.c, localbuf.c, buf_internals.h, pg_buffercache). A search of the heap access code found none of the symbols referenced below (heap_hot_indexed_create_tuple, heap_xlog_indexed_update, heap_hot_indexed_serialize_bitmap, HEAP_INDEXED_UPDATED). The commit therefore bundles unrelated content and is neither atomic nor bisectable. Confidence: high.
Comment on lines
+6
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. This entire file targets a "HOT indexed updates" feature whose symbols do not exist anywhere in this tree. I verified that |
||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||
|
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 the section headers (e.g. "HOT Indexed Updates — GDB breakpoints", "UPDATE DECISION — heap_update()"). PostgreSQL sources and diffs are required to be ASCII-only; use a plain hyphen/dash instead. Minor relative to the file not belonging in the tree at all, but noted for completeness. (high confidence)
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. Non-ASCII em-dash characters (U+2014) are used throughout the header comments (lines 1, 15, 38, 52, 81, 98, 106, 117, 140, 148). PostgreSQL sources must be ASCII-only - no em-dashes, smart quotes, or ellipsis characters. Replace
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. Non-ASCII em-dash characters (U+2014) appear in the header and every section separator (e.g. "HOT Indexed Updates —", "UPDATE DECISION —"). PostgreSQL source policy requires ASCII-only in source and diffs. Even setting aside that the whole file should be removed, this content would fail source-hygiene checks.
Suggested change
|
||||||||||||||||||||||||||||||
| # 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. Hardcoded file:line breakpoints are inaccurate and brittle. For example, 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 anchored to hardcoded 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) are inherently fragile: they silently drift as surrounding code changes and will land on the wrong statements. Since this file should not be committed at all (see above), this is moot, but even as a private aid, function-name breakpoints are far more robust than line-number ones.
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 inherently fragile: they silently drift the moment any adjacent code changes. In fact
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 file:line breakpoints are extremely fragile: they silently point at the wrong statement (or fail to resolve) after any edit to these files. Worse, these specific lines/symbols do not exist in the current tree - the referenced HOT-indexed functions and flag (heap_hot_indexed_*, heap_xlog_indexed_update, HEAP_INDEXED_UPDATED) were not found in src/backend/access/heap. These breakpoints reference code outside this change and are stale/meaningless here. 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. 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. These breakpoints reference HOT-indexed logic (heapam.c HOT-indexed decision, 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 line-number breakpoints are fragile: they drift the moment any line above them is edited, so gdb will silently stop at the wrong location without warning. The file already uses the maintainable form (function-name breakpoints like 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 (heapam.c:3210/4019/4024/4033/4101/4147, heapam_indexscan.c:182/250/297, indexam.c:299, execIndexing.c:370, pruneheap.c:1802/1836/1863/1287/2936) are extremely brittle: they become wrong the moment any line above them is added or removed, and the accompanying comments ('pure HOT', 'HEAP_INDEXED_UPDATED flag', 'redirect-with-data') are unverifiable against the tree since the corresponding feature code is not present. This reinforces that the file should be dropped rather than maintained. 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 hardcoded source line breakpoints do not match the current tree and reference nonexistent code. Verified against the repo:
Furthermore, the function-name breakpoints (
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 does not belong in a commitfest-bound PR. Two independent, verifiable defects make it actively misleading:
Per the top-level .gitignore policy, local-workflow/editor scaffolding should be ignored via
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. [high confidence] This developer-only debugging file must not be in a commitfest patch (see the local-workflow policy in 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 hardcoded source-line breakpoints are stale/wrong and actively misleading. Verified against the current tree:
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. The hardcoded absolute source line numbers ( 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 whole file targets a "HOT indexed updates" patch series (heapam.c / pruneheap.c / execIndexing.c / indexam.c / heapam_indexscan.c), but the actual change in this PR is a buffer-manager / clock-sweep replacement-policy change (bufmgr.c, freelist.c, localbuf.c, buf_internals.h, bufmgr.h). None of the files these breakpoints reference are modified here. Beyond the already-noted non-existent function breakpoints, the hardcoded file:line 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. These breakpoints target a "HOT indexed updates" patch series that is not present in this repository. Verified against the tree: the symbols
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 are brittle and have already drifted. 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. The referenced symbols heap_hot_indexed_create_tuple, heap_hot_indexed_tuple_size, heap_hot_indexed_serialize_bitmap, heap_xlog_indexed_update (and the rest of the heap_hot_indexed_* family) do not exist anywhere in the source, so these breakpoints are dead/misleading on commit. In addition, every file:line breakpoint (heapam.c:3210, pruneheap.c:2936, heapam_indexscan.c:182, etc.) is pinned to a hardcoded line number that drifts on any edit, making the file wrong almost immediately. This file is unrelated to the change in this update and should not be committed (see .clangd comment). 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 does not belong in a patch destined for pgsql-hackers/commitfest. It is a local, personal debugging aid and violates the minimal-diff / one-logical-change discipline. Beyond that, it is factually stale and out of scope for this buffer/NUMA review group:
Recommend dropping this file from the patch entirely.
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 debugging scaffolding that must not be committed to a PostgreSQL tree. The root Independently, the breakpoints are broken: I verified that |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # HOT decision block: pure HOT vs HOT indexed vs non-HOT | ||||||||||||||||||||||||||||||
| # Line 4019: pure HOT (no indexed columns changed) | ||||||||||||||||||||||||||||||
|
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 dead/misleading scaffolding. The symbols it breaks on ( 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 dead/misleading scaffolding. The symbols it breaks on ( |
||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||
|
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 file:line breakpoints (heapam.c:3210/4019/4024/4033/4101/4147, heapam_indexscan.c:182/250/297, indexam.c:299, execIndexing.c:370, pruneheap.c:1802/1836/1863/1287/2936) are extremely brittle: they drift with any unrelated edit and are meaningless outside one exact commit. The accompanying comments assert specific semantics per line (e.g. "Line 4019: pure HOT"), which become misleading documentation as the source evolves. None of these files even exist in this tree for this feature, so these references cannot be validated. This reinforces that the file does not belong in the patch. |
||||||||||||||||||||||||||||||
| break heapam.c:4024 | ||||||||||||||||||||||||||||||
| break heapam.c:4033 | ||||||||||||||||||||||||||||||
|
Comment on lines
+27
to
+29
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. All
Comment on lines
+27
to
+29
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: they drift with any edit to the target file and will silently point at unrelated code. For example Severity: high confidence.
Comment on lines
+27
to
+29
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. Confirmed stale/invalid line-number breakpoints. Line-number breakpoints rot the instant a file changes, and these are already wrong against the current tree: heapam.c:4019 is a deadlock-avoidance comment (not "pure HOT"), heapam.c:4024 is
Comment on lines
+27
to
+29
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 line-number breakpoints are already wrong against this tree. For example, the comment claims
Comment on lines
+27
to
+29
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 GDB breakpoints target a "HOT indexed updates" patch that is NOT present in this tree. Verified:
Comment on lines
+27
to
+29
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 absolute line-number breakpoints (
Comment on lines
+27
to
+29
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 GDB breakpoint file is WIP debug cruft and should not be committed. Two concrete defects prove it is already stale and unmaintainable: (1) The hardcoded line numbers are wrong --
Comment on lines
+27
to
+29
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 (e.g. |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # 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. This file targets a HOT-indexed-updates feature (
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 breakpoints reference an unrelated "HOT indexed updates" patch series that is not present in this tree. The symbols broken on here -- heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, heap_hot_indexed_read_bitmap, heap_xlog_indexed_update, etc. -- do not exist anywhere in the codebase (verified via search), so GDB will reject them. Combined with the hardcoded absolute line-number breakpoints below (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), which drift with any edit, this file is immediately stale and misleading. It is developer-local debug scaffolding that should not be committed. |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # 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 reference symbols that do not exist anywhere in this 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. These symbols do not exist anywhere in the tree. A codebase search for
Comment on lines
+43
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" feature that does not exist in this changeset. The symbols Severity: high confidence.
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.
Comment on lines
+42
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. This Worse, the breakpoints here target a completely different, nonexistent feature: the
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. This entire file is developer-local scaffolding and should not be committed to a patch destined for pgsql-hackers/commitfest. It is unrelated to the HOT-indexed-updates feature itself and violates the minimal-diff discipline. Beyond that, it is broken as committed: the function-name breakpoints reference symbols that do not exist in this tree. |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||
|
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. File-and-line breakpoints like 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. File-and-line breakpoints like
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. These file:line breakpoints are stale and misdescribed. In the current tree |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||
|
Comment on lines
+144
to
+145
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. Neither |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # ========================================================================= | ||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||
|
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 breakpoint targets an out-of-range line: pruneheap.c currently has only 2759 lines, so pruneheap.c:2936 cannot exist. This proves the hardcoded line references are stale and meaningless in the current tree. |
||||||||||||||||||||||||||||||
| 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 |
|---|---|---|
|
|
@@ -9,7 +9,8 @@ EXTENSION = pg_buffercache | |
| DATA = pg_buffercache--1.2.sql pg_buffercache--1.2--1.3.sql \ | ||
| pg_buffercache--1.1--1.2.sql pg_buffercache--1.0--1.1.sql \ | ||
| pg_buffercache--1.3--1.4.sql pg_buffercache--1.4--1.5.sql \ | ||
| pg_buffercache--1.5--1.6.sql pg_buffercache--1.6--1.7.sql | ||
| pg_buffercache--1.5--1.6.sql pg_buffercache--1.6--1.7.sql \ | ||
| pg_buffercache--1.7--1.8.sql | ||
|
Comment on lines
+12
to
+13
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 new SQL-callable functions pg_buffercache_partitions() and pg_buffercache_set_partition() introduce user-visible behavior but REGRESS is unchanged (still only pg_buffercache and pg_buffercache_numa). There is no regression test exercising the happy path or the error paths (non-1D array, NULL array element, out-of-range partition, mismatched weights length, debug_clocksweep_balance_recalc enabled). A user-visible change with no tests is WIP, not commit-ready; please add sql/expected cases and list them here (and in meson.build). |
||
| PGFILEDESC = "pg_buffercache - monitoring of shared buffer cache in real-time" | ||
|
|
||
| REGRESS = pg_buffercache pg_buffercache_numa | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||||||||||||||||||||||||||||||||||||||
| -- complain if script is sourced in psql, rather than via CREATE EXTENSION | ||||||||||||||||||||||||||||||||||||||||||
|
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 an upgrade script, so the leading comment should refer to ALTER EXTENSION ... UPDATE (as the \quit message below does and as 1.6--1.7 does), not CREATE EXTENSION.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| \echo Use "ALTER EXTENSION pg_buffercache UPDATE TO '1.8'" to load this file. \quit | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| -- Register the new functions. | ||||||||||||||||||||||||||||||||||||||||||
| CREATE OR REPLACE FUNCTION pg_buffercache_partitions() | ||||||||||||||||||||||||||||||||||||||||||
|
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 plain CREATE FUNCTION, not CREATE OR REPLACE, in an extension upgrade script that introduces a brand-new function. OR REPLACE masks accidental name collisions and diverges from the established convention in this extension's upgrade scripts (e.g. 1.6--1.7 uses plain CREATE FUNCTION for all new functions).
Suggested change
Comment on lines
+4
to
+5
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 introduces two user-visible SQL-callable functions and a view but ships without documentation or regression tests. doc/src/sgml/pgbuffercache.sgml has no entry for pg_buffercache_partitions / pg_buffercache_set_partition (no '1.8' reference found there), and REGRESS in the Makefile / meson.build still only lists pg_buffercache and pg_buffercache_numa, so nothing exercises the new functions or their error paths (e.g. the non-one-dimensional array and NULL-element rejections, partition-out-of-range). Per contribution standards a user-visible change needs both docs and tests before it is commit-ready.
Comment on lines
+4
to
+5
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. No documentation accompanies this feature. The two new functions/views (pg_buffercache_partitions, pg_buffercache_set_partition) are absent from doc/src/sgml, and the usagecount semantics changed in pg_buffercache_pages.c (now 0/1 via BUF_STATE_HAS_SECOND_CHANCE, and pg_buffercache_usage_counts now returns only 2 rows instead of BM_MAX_USAGE_COUNT+1). That output change is backward-incompatible for existing monitoring queries and must be documented and justified. Please add SGML docs for the new objects and the changed usagecount semantics. |
||||||||||||||||||||||||||||||||||||||||||
| RETURNS SETOF RECORD | ||||||||||||||||||||||||||||||||||||||||||
| AS 'MODULE_PATHNAME', 'pg_buffercache_partitions' | ||||||||||||||||||||||||||||||||||||||||||
| LANGUAGE C PARALLEL SAFE; | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
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. New user-visible SQL objects (pg_buffercache_partitions view/function and pg_buffercache_set_partition) ship without regression tests or documentation. By PostgreSQL standards this is WIP, not commit-ready: add coverage in contrib/pg_buffercache/sql + expected (and extend the Makefile REGRESS list / meson regress sql list), and document the new view/functions in doc/src/sgml/pgbuffercache.sgml, including error paths (e.g. setting weights while debug_clocksweep_balance_recalc is enabled, out-of-range partition, wrong-length/NULL weights).
Comment on lines
+5
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. Use plain CREATE FUNCTION rather than CREATE OR REPLACE FUNCTION here. This is a fresh function being introduced in an extension upgrade script (1.7->1.8); the object cannot pre-exist, so OR REPLACE is unnecessary and inconsistent with the pattern used by the sibling pg_buffercache_set_partition below and by prior upgrade scripts (e.g. 1.6->1.7). OR REPLACE would also silently mask an accidental duplicate definition. Confidence: moderate.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| -- Create a view for convenient access. | ||||||||||||||||||||||||||||||||||||||||||
| CREATE VIEW pg_buffercache_partitions AS | ||||||||||||||||||||||||||||||||||||||||||
|
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. No documentation and no regression tests accompany these new SQL-callable objects. The view |
||||||||||||||||||||||||||||||||||||||||||
| SELECT P.* FROM pg_buffercache_partitions() AS P | ||||||||||||||||||||||||||||||||||||||||||
| (partition integer, -- partition index | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+13
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. Indentation uses tabs here, but the other pg_buffercache SQL scripts (e.g. 1.6--1.7) indent with 4 spaces. Keep the new file consistent with the existing files to avoid gratuitous whitespace divergence. |
||||||||||||||||||||||||||||||||||||||||||
| numa_node integer, -- NUMA node of the partitioon | ||||||||||||||||||||||||||||||||||||||||||
|
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 typo: "partitioon" should be "partition".
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. Spelling: "partitioon" should be "partition".
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. Typo in user-visible column comment: "partitioon" should be "partition".
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. Typo in user-visible column comment: "partitioon" should be "partition".
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. Typo in a user-facing catalog comment: "partitioon" should be "partition". [high confidence]
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. Typo in comment: "partitioon" should be "partition". Confidence: high.
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. Typo in comment: "partitioon" should be "partition".
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. Typo in comment: "partitioon" should be "partition".
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. Typo in comment: "partitioon" should be "partition". Confidence: high.
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. Typo in user-facing column comment: "partitioon" -> "partition".
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| num_buffers integer, -- number of buffers in the partition | ||||||||||||||||||||||||||||||||||||||||||
| first_buffer integer, -- first buffer of partition | ||||||||||||||||||||||||||||||||||||||||||
| last_buffer integer, -- last buffer of partition | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| -- clocksweep counters | ||||||||||||||||||||||||||||||||||||||||||
| num_passes bigint, -- clocksweep passes | ||||||||||||||||||||||||||||||||||||||||||
| next_buffer integer, -- next victim buffer for clocksweep | ||||||||||||||||||||||||||||||||||||||||||
| total_allocs bigint, -- handled allocs (running total) | ||||||||||||||||||||||||||||||||||||||||||
| num_allocs bigint, -- handled allocs (current cycle) | ||||||||||||||||||||||||||||||||||||||||||
| total_req_allocs bigint, -- requested allocs (running total) | ||||||||||||||||||||||||||||||||||||||||||
| num_req_allocs bigint, -- handled allocs (current cycle) | ||||||||||||||||||||||||||||||||||||||||||
|
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 copy-paste error: num_req_allocs is "requested allocs (current cycle)", not "handled allocs (current cycle)" (which is num_allocs on line 23). Fix the comment to match the SQL view in pg_buffercache_pages.c, which exposes buffer_req_allocs here.
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. Comment is wrong/copy-pasted:
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. The comment for num_req_allocs is copy-pasted from num_allocs ("handled allocs (current cycle)"). Based on the C code (buffer_req_allocs from numRequestedAllocs) and the total_req_allocs comment above, this should read "requested allocs (current cycle)".
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. The comment for num_req_allocs is copy-pasted from num_allocs ("handled allocs (current cycle)"). Based on the C code (buffer_req_allocs from numRequestedAllocs) and the total_req_allocs comment above, this should read "requested allocs (current cycle)".
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. Comment appears to be a copy-paste from num_allocs: num_req_allocs is the requested-allocs counter for the current cycle, but the comment says "handled allocs (current cycle)", duplicating the num_allocs description. Should read "requested allocs (current cycle)". [high confidence]
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. Copy-paste error in the column comment: Confidence: high.
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. Comment is a copy-paste error:
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. Comment is a copy-paste error: num_req_allocs is the requested-allocs counter for the current cycle (num_allocs is the handled one). The comment should read "requested allocs (current cycle)", not "handled allocs (current cycle)".
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. Incorrect comment: num_req_allocs is populated from buffer_req_allocs (requested allocs in the current cycle), so the comment should read "requested allocs (current cycle)", not "handled allocs (current cycle)" (copy-paste from num_allocs). Confidence: high.
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. Copy-paste error in the column comment: num_req_allocs is the requested-alloc counter for the current cycle, but the comment reads "handled allocs" (duplicated from num_allocs above). Fix to describe requested allocs.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| weights int[]); -- balancing weights | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| -- Register the function to set clock-sweep balance weights. | ||||||||||||||||||||||||||||||||||||||||||
| CREATE FUNCTION pg_buffercache_set_partition(IN partition int, IN weights int[]) | ||||||||||||||||||||||||||||||||||||||||||
|
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. No regression tests and no documentation accompany these new user-callable objects. REGRESS (Makefile) and meson.build still list only pg_buffercache and pg_buffercache_numa, no expected/*.out files were added for 1.8, and there is no pg_buffercache SGML doc update for the new pg_buffercache_partitions view or pg_buffercache_set_partition() function. Per project rules a user-visible change without tests + docs is WIP, not commit-ready; the error paths in ClockSweepSetWeights (out-of-range partition, weight-count mismatch, out-of-range weight, recalc-enabled) in particular need coverage. [high confidence] |
||||||||||||||||||||||||||||||||||||||||||
| RETURNS void | ||||||||||||||||||||||||||||||||||||||||||
| AS 'MODULE_PATHNAME', 'pg_buffercache_set_partition' | ||||||||||||||||||||||||||||||||||||||||||
| LANGUAGE C VOLATILE PARALLEL UNSAFE; | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+28
to
+32
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 upgrade adds two user-visible SQL functions and a view but ships no regression tests and no documentation. The Makefile REGRESS target (pg_buffercache, pg_buffercache_numa) is unchanged, there is no partitions test under sql/expected, and doc/src/sgml/pgbuffercache.sgml has no mention of partitions. Per PostgreSQL commit-readiness standards, a user-visible change needs regression tests (covering the happy path plus error paths like out-of-range partition, mismatched weights length, NULL/multi-dim array for set_partition) and documentation for the new functions and view. Without them this is WIP.
Comment on lines
+28
to
+32
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 upgrade adds two user-visible SQL functions and a view but ships no regression tests and no documentation. The Makefile REGRESS target (pg_buffercache, pg_buffercache_numa) is unchanged, there is no partitions test under sql/expected, and doc/src/sgml/pgbuffercache.sgml has no mention of partitions. Per PostgreSQL commit-readiness standards, a user-visible change needs regression tests (covering the happy path plus error paths like out-of-range partition, mismatched weights length, NULL/multi-dim array for set_partition) and documentation for the new functions and view. Without them this is WIP.
Comment on lines
+29
to
+32
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. User-visible feature without documentation or regression coverage. The new pg_buffercache_partitions view/function and pg_buffercache_set_partition (a knob that mutates shared clock-sweep balance state) have no SGML docs (doc/src/sgml/pgbuffercache.sgml is not updated) and no regression tests (nothing in sql/ or expected/ references pg_buffercache_partitions or pg_buffercache_set_partition). Per PostgreSQL standards a user-visible SQL addition must ship with docs plus tests covering happy, edge, and error paths (e.g. out-of-range partition, wrong-length weights array, NULL array elements, and the clocksweep_balance_recalc-enabled rejection in ClockSweepSetWeights). Confidence: high. |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| -- Don't want these to be available to public. | ||||||||||||||||||||||||||||||||||||||||||
| REVOKE ALL ON FUNCTION pg_buffercache_partitions() FROM PUBLIC; | ||||||||||||||||||||||||||||||||||||||||||
| REVOKE ALL ON pg_buffercache_partitions FROM PUBLIC; | ||||||||||||||||||||||||||||||||||||||||||
| REVOKE ALL ON FUNCTION pg_buffercache_set_partition(int, int[]) FROM PUBLIC; | ||||||||||||||||||||||||||||||||||||||||||
|
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. pg_buffercache_set_partition is only REVOKEd from PUBLIC with no role grant, so by default it is superuser-only; that is appropriate for a function that mutates shared buffer-manager state. However, there is no regression test exercising this new SQL surface: neither pg_buffercache_partitions nor the set_partition error paths (non-1-D array, NULL element, out-of-range partition, weight out of 0..100) are covered, and there is no pgbuffercache.sgml documentation for the two new functions in this change. A user-visible SQL addition without tests and docs is incomplete. 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. pg_buffercache_set_partition is REVOKE'd from PUBLIC but never GRANTed to any role, so after this upgrade only superusers (and the extension owner) can execute it. If that is the intended access model, this is fine; but note pg_buffercache_partitions is granted to pg_monitor while the mutating setter is left superuser-only without comment. Confirm this asymmetry is deliberate and document it. (Not a privilege-escalation issue: the default REVOKE correctly denies PUBLIC.) 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. pg_buffercache_set_partition is REVOKE'd from PUBLIC but never GRANTed to any role, so after this upgrade only superusers (and the extension owner) can execute it. If that is the intended access model, this is fine; but note pg_buffercache_partitions is granted to pg_monitor while the mutating setter is left superuser-only without comment. Confirm this asymmetry is deliberate and document it. (Not a privilege-escalation issue: the default REVOKE correctly denies PUBLIC.) |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| GRANT EXECUTE ON FUNCTION pg_buffercache_partitions() TO pg_monitor; | ||||||||||||||||||||||||||||||||||||||||||
| GRANT SELECT ON pg_buffercache_partitions TO pg_monitor; | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # pg_buffercache extension | ||
| comment = 'examine the shared buffer cache' | ||
| default_version = '1.7' | ||
| default_version = '1.8' | ||
| module_pathname = '$libdir/pg_buffercache' | ||
| relocatable = true |
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
.clangdfile is personal editor/LSP tooling configuration and should not be part of a patch destined for pgsql-hackers/commitfest. Per PostgreSQL patch hygiene, a patch must be a minimal diff containing only what the stated change requires; committing local IDE/tooling config (this file, plus.gdbinitandpg-aliases.shin this same change) is unrelated churn and a reliable rejection reason. The PostgreSQL tree does not carry.clangd. If you need it locally, put it in.git/info/excludeor your global gitignore rather than committing it. Remove this file from the patch.Also note the hardcoded relative include path
-I../../../../src/include(line 43) is brittle and workstation/subdirectory-specific, which further confirms this is a local artifact, not a tree-wide config.