Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .clangd
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
Diagnostics:
MissingIncludes: None
Comment on lines +1 to +2

Copy link
Copy Markdown

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.

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file (and the sibling .envrc, .gdbinit, flake.nix, shell.nix, pg-aliases.sh) is personal developer-environment scaffolding. None of it belongs in a PostgreSQL patch destined for pgsql-hackers/commitfest: it is not part of the stated change (the HOT indexed-updates series in the other files), it hardcodes one contributor's layout (Nix, direnv, a specific build/ dir, -I../../../../src/include), and it will draw an immediate rejection as unrelated churn. PostgreSQL already provides src/tools/editors/ and in-tree meson/autoconf build support; adding a top-level .clangd imposes an editor/toolchain choice on the whole project. Drop these files from the patch (keep them in a local, git-ignored overlay instead).

Comment on lines +1 to +2

Copy link
Copy Markdown

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 tooling committed to the repo root. For a fork whose PRs become pgsql-hackers patches, these are unrelated-change noise that violates the minimal/atomic-diff discipline and will draw immediate rejection.

Critically, the fork's own .github/workflows/fork-sync-upstream.yml rebase policy errors out on any master file other than .github/workflows/fork-*.yml (grep -v '^\.github/workflows/fork-'). These root-level dev files are exactly what that guard rejects, so they do not belong on master. They should live on a feature branch or in a personal (git-ignored) location, not in a patch.

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 $GIT_DIR/info/exclude or ~/.gitexclude rather than committed. These should be dropped from the patch destined for pgsql-hackers.

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Developer-local editor configuration. It pins personal include paths (-I../../../../src/include) and a build/ compile-commands dir that only make sense in the author's worktree layout. This does not belong in a PostgreSQL patch destined for pgsql-hackers; it is non-minimal, non-portable clutter that will cause needless churn. Track it via a local git-excludes file instead.

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high confidence] This file should not be committed. .clangd, .envrc, .gdbinit, pg-aliases.sh, flake.nix, and shell.nix are all personal developer tooling with no bearing on the HOT-indexed-updates feature. A patch destined for pgsql-hackers/commitfest must be minimal and contain only files the feature requires; a local Nix dev shell, direnv config, and a .gdbinit of personal breakpoints are exactly the kind of unrelated churn that gets a patch rejected on the list. The tree's own root .gitignore already documents the convention: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." Keep these out of the commit (local exclude), rather than in the tree.

Additionally, -DPGDLLIMPORT= and the extra warning flags (-Wconversion, -Wfloat-equal, -Wshadow, -Werror=vla) diverge from PostgreSQL's standard meson build and will produce misleading local diagnostics; redefining the ABI-critical PGDLLIMPORT macro even just for clangd is a footgun.

Comment on lines +1 to +2

Copy link
Copy Markdown

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/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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 (.gitignore lines 1-3) that "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude" rather than committed. Drop .clangd, .envrc, .gdbinit, pg-aliases.sh, flake.nix, and shell.nix from the patch. (high confidence)

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.clangd is per-developer editor configuration and does not belong in the repository. PostgreSQL's root .gitignore directs such local-workflow files to $GIT_DIR/info/exclude or ~/.gitexclude. The include paths here (-I../../../../src/include) are also relative to a specific build-tree subdirectory and will not resolve correctly from the repo root, making this fragile outside the author's setup.

Comment on lines +1 to +2

Copy link
Copy Markdown

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 scaffolding, unrelated to any functional change. The repo's own .gitignore header states: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." Committing these to the tree violates the minimal-diff / one-logical-change discipline and will draw immediate rejection on pgsql-hackers. They should be removed from the patch and kept local. [high confidence]

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This .clangd file is personal editor/LSP scaffolding (it hardcodes build/ as the compilation database and a developer-specific warning set). It is unrelated to any PostgreSQL feature and should not be committed to the source tree; a patch posted to pgsql-hackers should contain only the minimal diff for the stated change. Keep this in a local (untracked) config or .gitignore it instead.

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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. CompilationDatabase: build/, a fixed -I../../../../src/include relative path). If you need local editor config, cover it via a local gitignore (the shell hook already sets core.excludesFile .local-gitignore) rather than committing it.

[high confidence]

Comment on lines +1 to +2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 developer-local scaffolding unrelated to the HOT indexed-updates feature. The root .gitignore explicitly 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 and bundles unrelated concerns into the feature patch; this will be an immediate rejection on pgsql-hackers. Move them to a personal dotfiles/local-ignore, not the patch.

InlayHints:
Enabled: true
ParameterNames: true
DeducedTypes: true
CompileFlags:
CompilationDatabase: build/ # Search build/ directory for compile_commands.json

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CompilationDatabase: build/ and the trailing -I../../../../src/include hardcode one developer's directory layout (a build/ dir and a specific nesting depth). This won't match other contributors' setups and reinforces that this is personal, non-portable config that should not be committed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 -I. and -I../../../../src/include and CompilationDatabase: build/ are tied to one author's worktree layout and will not resolve for other contributors. Remove from the patch; keep it under a local exclude mechanism.

Confidence: high.

Comment on lines +7 to +8

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These include hacks (-I., -I../../../../src/include) and CompilationDatabase: build/ are workstation-specific and will not match other contributors' layouts; MissingIncludes: None additionally masks real include errors. This is editor-only config that should not ship in a pgsql-hackers patch.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding CompilationDatabase: build/ and the relative include -I../../../../src/include ties this editor config to one specific build-directory name and source-tree depth. It will be wrong for any contributor using a different build layout, and like the other files here it is personal environment tooling that should be excluded locally rather than committed.

Comment on lines +7 to +8

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 (CompilationDatabase: build/, -I../../../../src/include relative paths that only resolve from a specific subdirectory) and disables project warnings (Remove: [ -Werror ]). This is not part of the HOT-indexed-update feature and belongs in the developer's local-only ignore (shell.nix already sets core.excludesFile .local-gitignore).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 CompilationDatabase: build/, this is tightly coupled to one developer's worktree layout and won't work for others; another reason this config does not belong in the shared tree.

Remove: [ -Werror ]
Add:
- -DDEBUG
- -DLOCAL
- -DPGDLLIMPORT=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out-of-scope developer artifact. This .clangd is editor/LSP configuration unrelated to the HOT indexed-updates feature and should not be tracked in the PostgreSQL tree (see root .gitignore: local-workflow files belong in $GIT_DIR/info/exclude or ~/.gitexclude). Separately, -DPGDLLIMPORT= silently blanks the macro for the IDE; since PGDLLIMPORT is the Windows/MSVC export gate, this diverges from real build semantics and would mask IDE diagnostics about missing export markers on any new exported externs the series introduces.

- -DPIC
Comment on lines +13 to +14

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-DPGDLLIMPORT= blanks out the Windows export annotation in the editor. Reviewers rely on seeing PGDLLIMPORT to verify that extern variables crossing module/extension boundaries are correctly exported for MSVC; masking it defeats exactly the check this project needs. Combined with -Wconversion/-Wfloat-equal/-Wshadow (not in PostgreSQL's real flag set), this surfaces a flood of false-positive diagnostics that do not reflect the buildfarm.

- -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The include path -I../../../../src/include is hardcoded to a specific nesting depth (four levels up), which only resolves if this repo lives at exactly that relative position. Combined with -I., header resolution for clangd will be wrong for anyone whose checkout layout differs. Since clangd already consumes compile_commands.json from build/, these manual -I flags are both redundant and fragile.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The include path -I../../../../src/include is hard-coded to a fixed nesting depth, so it only resolves for source files exactly four directories below the repo root. Files at other depths (e.g. src/backend/postgres.c, top-level, or deeper under src/backend/access/heap/) will fail header resolution in clangd. Rely on compile_commands.json (already configured via CompilationDatabase: build/) for include paths rather than a fixed relative -I.

Comment on lines +42 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The include path -I../../../../src/include is a hardcoded relative-depth hack that only resolves correctly when clangd is invoked from a file nested exactly four directories deep (e.g. src/backend/access/heap/*.c). From the repo root or any other depth it points outside the tree and resolves nothing, so cross-file navigation/diagnostics silently break. Beyond that, a committed .clangd is personal editor scaffolding that does not belong in the patch.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The include path -I../../../../src/include is wrong for this repo. src/include lives at the repository root (verified: src/include/postgres.h exists), so relative to .clangd at the tree root it is ./src/include, not four levels up. As written this resolves to a path four directories above the repo root and will not provide PostgreSQL headers to clangd.

Suggested change
- -I../../../../src/include
+ - -Isrc/include

10 changes: 10 additions & 0 deletions .envrc
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 /.gitexclude, not committed. This file in particular hardcodes a private path (/ws/postgres/.envrc) and references non-public direnv extensions (use project_steering, use project_mcp, use git_policy allow-force-push) that are meaningless to anyone else and will not evaluate. A pgsql-hackers patch must be minimal; drop all of these from the submission.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This direnv config leaks the author's local workflow: a hardcoded ~/ws/postgres/.envrc path via source_up, and project-specific direnv helpers (use project_steering, use project_mcp postgresq [sic -- also looks truncated/typo'd], use git_policy allow-force-push) that no other contributor has. This is environment-specific tooling that should not be in a patch destined for pgsql-hackers; keep it in a local, gitignored location.

watch_file flake.nix
use flake
Comment on lines +4 to +6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Developer-local direnv file referencing personal paths (~/ws/postgres/.envrc) and private helpers (source_up, pg_cflags, use project_steering ..., use project_mcp postgresq). This is personal workflow scaffolding that does not belong in a pgsql-hackers patch; the root .gitignore documents ignoring such files via $GIT_DIR/info/exclude. Note also use project_mcp postgresq looks like a truncated/typo'd argument. Remove from the patch.

Confidence: high.

Comment on lines +4 to +6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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: source_up into ~/ws/postgres/.envrc, and custom direnv extensions use project_steering, use git_policy, use project_mcp. None of these are reproducible on another machine or relevant to the HOT-indexed-update feature. shell.nix already configures core.excludesFile .local-gitignore, indicating these files are meant to be local-only; they should be ignored rather than tracked.

Comment on lines +4 to +6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 (~/ws/postgres/.envrc) and depends on custom, undefined direnv helpers (pg_cflags, use project_steering ..., use git_policy ..., use project_mcp ...) that no other developer has, so direnv allow will fail for everyone else. Note also the apparent typo on the last line: project_mcp postgresq (missing trailing l). Remove this file from the patch.

[high confidence]

pg_cflags
Comment on lines +4 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.envrc (direnv) is personal environment scaffolding and should not be committed. The repo's root .gitignore explicitly directs such local-workflow files to $GIT_DIR/info/exclude or ~/.gitexclude. It also references external, machine-specific resources (source_up pulling ~/ws/postgres/.envrc, custom helpers pg_cflags, use project_steering, use project_mcp postgresq) that only exist on the author's setup, so it cannot work for anyone else. For a fork whose PRs become pgsql-hackers patches this is out-of-scope churn.

use project_steering postgresql aws c
use git_policy allow-force-push
use project_mcp postgresq
Comment on lines +4 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.envrc depends on a developer-personal, non-reproducible environment: source_up pulls in an absolute host path (~/ws/postgres/.envrc per the comment), and pg_cflags, use project_steering, use git_policy, and use project_mcp are custom direnv functions that only exist in one developer's setup. This file will fail to load (direnv: ... command not found) for anyone else and is not something a PostgreSQL patch should carry. Also use project_mcp postgresq appears to be a typo for postgresql.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 10 has a typo: project_mcp postgresq is missing the trailing l (should be postgresql), so this use directive will fail to locate the intended context. More broadly, this file depends on a parent ~/ws/postgres/.envrc (via source_up) and custom direnv helpers (pg_cflags, use project_steering, use git_policy, use project_mcp) that exist only in one developer's personal direnv library -- it is non-portable and should not be committed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use project_mcp postgresq looks like a typo (postgresq vs postgresql), which would make this direnv directive silently fail or error. More importantly, .envrc references machine-specific context (~/ws/postgres/.envrc, source_up, custom use project_steering/use git_policy/use project_mcp helpers) that only exist in the author's environment and cannot be reproduced by anyone else, so this file has no place in the committed tree.

Comment on lines +8 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This .envrc depends on personal direnv stdlib helpers (source_up, pg_cflags, use project_steering, use git_policy allow-force-push, use project_mcp) that are not present in the repo, so direnv will error out for anyone else. Also use project_mcp postgresq looks like a truncated/typo'd argument ("postgresq"). This is leaked personal config and should not be committed; per the repo's own .gitignore (lines 1-3), local-workflow files belong in $GIT_DIR/info/exclude or ~/.gitexclude, not in the tree.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These direnv helpers (source_up, pg_cflags, use project_steering ..., use git_policy ..., use project_mcp ...) depend on a private global direnv library and a parent ~/ws/postgres/.envrc that no one else has; anyone else entering the repo gets direnv errors. This file is machine-specific and non-reproducible. Also note postgresq on the last line is a truncated typo (missing trailing l), which would break the helper even on the author's own machine.

Comment on lines +8 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This direnv file sources machine-specific, author-local helpers (source_up pulling ~/ws/postgres/.envrc, use project_steering postgresql aws c, use project_mcp postgresq) that are not reproducible outside the author's machine. The use project_mcp postgresq argument also looks truncated/misspelled (postgresq vs postgresql). This file must not be committed.

Comment on lines +4 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high confidence] Non-portable personal config. source_up reaches into a personal ~/ws/postgres/.envrc, and pg_cflags, use project_steering postgresql aws c, and use project_mcp postgresq depend on private direnv library functions that are not defined anywhere in this repo (grep for project_steering/project_mcp matches only this file). Committed, this file fails direnv allow for every other developer and leaks a personal workspace layout. Note also the typo postgresq (missing trailing l). This file should be kept local, not committed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use project_mcp postgresq looks like a typo for postgresql. More importantly, source_up and the custom use project_steering/use git_policy allow-force-push/use project_mcp helpers depend on direnv extensions and parent-directory configuration that are not reproducible for other developers; this file cannot evaluate outside the author's machine and should not be in the tree.

Comment on lines +8 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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, use project_mcp postgresq appears to be a truncated/typo'd argument ("postgresq" vs "postgresql"). This is a personal-config leak and is non-portable. (high confidence)

Comment on lines +4 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file depends on the author's personal ~/ws/postgres/.envrc (source_up) and custom direnv stdlib helpers (pg_cflags, use project_steering, use git_policy allow-force-push, use project_mcp postgresq). Confirmed none of these helpers are defined anywhere in the repository, so the file is non-portable and meaningless to any other contributor. use project_mcp postgresq also looks like a truncated typo (postgresq vs postgresql). Remove this file from the patch. (high confidence)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

postgresq is a truncated/misspelled argument (presumably postgresql). Combined with the other undefined direnv helpers, this will likely cause the use project_mcp call to fail or load the wrong context.

Suggested change
use project_mcp postgresq
use project_mcp postgresql

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely typo in the direnv extension name: use project_mcp postgresq (missing trailing l, cf. the DB name used elsewhere as postgresql). If this is meant to match a named profile it will silently load the wrong/no profile.

Comment on lines +4 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This .envrc is unusable outside the author's machine: source_up resolves to a hardcoded ~/ws/postgres/.envrc (per the comment), and pg_cflags / use project_steering / use git_policy / use project_mcp are custom direnv helpers not shipped in this repo. Also note the apparent truncation/typo project_mcp postgresq (missing trailing 'l'). Another reason this personal direnv config does not belong in the committed tree.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely typo: project_mcp postgresq is missing the trailing l (should presumably be postgresql), inconsistent with use project_steering postgresql two lines above.

Comment on lines +4 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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. source_up pulls CFLAGS from a hardcoded absolute path ~/ws/postgres/.envrc that exists only on the author's machine; pg_cflags, use project_steering postgresql aws c, use git_policy allow-force-push, and use project_mcp postgresq are all custom direnv helpers / stdlib extensions that do not exist in this repo or a stock direnv, so direnv allow fails for everyone else. Note line 10 is also truncated (project_mcp postgresq, missing trailing l). Drop this file from the patch (and gitignore it locally).

Comment on lines +4 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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: source_up reaches into ~/ws/postgres/.envrc (hardcoded home path), and pg_cflags, use project_steering, use git_policy, use project_mcp are custom direnv library functions not shipped in this repo. For any other contributor direnv allow will error out. This is personal scaffolding that should be git-ignored, not committed to a patch headed for pgsql-hackers.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparent typo: postgresq is missing a trailing l (should likely be postgresql). An invalid argument to use project_mcp will fail or silently misconfigure the context.

Suggested change
use project_mcp postgresq
use project_mcp postgresql

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out-of-scope developer artifact. This .envrc is a personal direnv configuration (references ~/ws/postgres, custom use project_steering/use git_policy/use project_mcp helpers that only exist in the author's setup) and is unrelated to the feature. It must not be committed to the tree; ignore it locally instead. Note the trailing project_mcp postgresq also looks truncated/typo'd.

156 changes: 156 additions & 0 deletions .gdbinit
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
# HOT Indexed Updates — GDB breakpoints for code review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These breakpoints reference code that does not exist in this change set. heap_xlog_indexed_update and every heap_hot_indexed_* helper (e.g. heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, heap_hot_indexed_read_bitmap, heap_hot_indexed_merge_bitmaps_raw) are not defined anywhere in the tree -- they belong to a not-yet-merged "HOT indexed updates" series. A committed .gdbinit that breaks on non-existent symbols is dead/aspirational content. This file is personal debugging tooling and should be kept local (the repo's own .gitignore says local-workflow files belong in $GIT_DIR/info/exclude or ~/.gitexclude), not committed to the tree.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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
# HOT Indexed Updates — GDB breakpoints for code review
# HOT Indexed Updates -- GDB breakpoints for code review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out-of-scope developer artifact. This .gdbinit is personal debugging scaffold for a HOT indexed-updates patch series and does not belong in the PostgreSQL tree. The root .gitignore explicitly documents the intended workflow: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." Committing per-developer tooling is a top rejection reason on pgsql-hackers. This file (and .clangd, .envrc, flake.nix, shell.nix, pg-aliases.sh) should be ignored locally, not tracked.

#
# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 lp = PageGetItemId(page, offnum);, which does not match the comment's claim of "initialize bitmap accumulator from collapsed redirect" -- the line-number breakpoints are already stale. More fundamentally, this entire file is personal developer tooling that should not be committed to a PostgreSQL patch destined for pgsql-hackers/commitfest; it violates the minimal-diff/one-logical-change discipline. Move it to a local (git-ignored) location. If kept, prefer symbol-based breakpoints (e.g. break heap_prune_chain) which survive edits.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These absolute source-line breakpoints (e.g. heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299, heapam_indexscan.c:182) are extremely fragile: they drift with every unrelated edit and are tied to a specific in-progress "HOT indexed updates" branch. As a committed artifact they will silently point at the wrong lines. This file should not be in the tree at all (local-only tooling), but at minimum line-number breakpoints are unmaintainable.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This .gdbinit pins breakpoints to absolute source line numbers, and they are already wrong/fragile. Verified against the current tree: heapam.c:4019 is a RelationGetBufferForTuple() call (not the claimed "pure HOT" decision), heapam.c:4024 is a VM-pin, heapam.c:4033 is a comment; heapam_indexscan.c:182 is lp = PageGetItemId(...) (not a "bitmap accumulator"), :250 is skip = false;, and :297 is a return inside an unrelated function. Any edit above these lines silently moves the breakpoint to the wrong code -- a debugging footgun. Line-number breakpoints like these should not be committed; use symbol-based breakpoints at minimum, and preferably keep this file out of the tree entirely (see the scope concern below).

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolute line-number breakpoints are extremely fragile and already wrong. Verified heapam.c:3210 is the closing brace of heap_update's return TM_Ok; } region, not the "Main entry: heap_update" claimed by the comment. All the numeric breakpoints (heapam.c:4019/4024/4033/..., heapam_indexscan.c:182, pruneheap.c:1802, indexam.c:299, execIndexing.c:370) drift with any edit and cannot be trusted.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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: break heapam.c:3210 is claimed to be "Main entry: heap_update", but heapam.c:3210 is the closing brace of heap_delete. Line-pinned breakpoints silently bind to unrelated code the moment any line shifts, so the file is actively misleading even to its author.

Furthermore, the symbols this file documents do not exist anywhere in the tree: heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, XLOG_HEAP2_INDEXED_UPDATE, and heap_xlog_indexed_update appear ONLY inside .gdbinit. This couples an infrastructure/tooling commit to unlanded feature commits, violating the atomic/bisectable-commit rule: the file documents code that is not present in this change.

Confidence: high.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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. break heap_hot_search_buffer) or drop the file entirely since it is personal tooling.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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. heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299) and symbol breakpoints (heap_hot_indexed_create_tuple, heap_hot_indexed_tuple_size, heap_xlog_indexed_update, ...) for an unrelated "HOT indexed updates" patch series. None of these functions exist in this review group (searching the tree finds them only inside this .gdbinit), so the breakpoints are both unrelated to any file here and immediately stale. Line-number breakpoints also drift the instant surrounding code changes. This file should be in a local git-excludes file, not tracked.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line-number breakpoints are fragile and already wrong. heapam.c:3210 is not the entry of heap_update(): in the current tree line 3210 is the closing brace of a function epilogue (return TM_Ok; }), so this break lands on the wrong statement. Any edit above these lines silently drifts every file:line break here (heapam.c:4019/4024/4033/4101/4147, pruneheap.c:1802/1836/1863/1287/2936, indexam.c:299, heapam_indexscan.c:182/250/297, execIndexing.c:370). Prefer symbol-based breakpoints over hardcoded line numbers.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 (heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299, execIndexing.c:370, heapam_indexscan.c:182, etc.) silently shift onto the wrong statement after any edit to those files -- a debugging footgun that rots immediately. Beyond that, several symbolic breakpoints here reference functions that do not exist anywhere in this tree: heap_hot_indexed_create_tuple, heap_hot_indexed_tuple_size, heap_hot_indexed_read_bitmap, and heap_xlog_indexed_update have no definitions in any .c/.h (grep returns only this file). So the file is already out of sync with the code it claims to debug. This is personal tooling that should not be committed (see .gitignore convention); keep it local via $GIT_DIR/info/exclude.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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. heapam.c:3210 is annotated as the heap_update entry, but line 3210 is actually the closing brace of heap_delete. Similarly heapam_indexscan.c:182 is annotated as "initialize bitmap accumulator from collapsed redirect", but that line is a plain PageGetItemId() call and the bitmap-accumulator logic does not exist in the tree. Line-number breakpoints are inherently fragile and should never be committed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breakpoints are encoded as hardcoded source line numbers (e.g. heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299) and symbol names (heap_hot_indexed_*, heap_xlog_indexed_update, heap_hot_search_buffer, heap_page_prune_and_freeze). None of those functions exist in this repository, and heapam_indexscan.c referenced at lines 182/250/297 is only 588 lines with unrelated content, so the line numbers are already stale. A debugging aid that drifts the moment any file changes is unreliable by construction and is not something the project commits. Remove this file from the patch. (high confidence)

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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:

  1. It encodes hardcoded absolute source line-number breakpoints (heapam.c:3210, heapam.c:4019, heapam.c:4147, pruneheap.c:1802, execIndexing.c:370, indexam.c:299, etc.). These drift the instant any surrounding code changes, so they couple the file to one specific WIP revision and are unmaintainable.
  2. It references functions that do not exist in the tree: a search for heap_hot_indexed_create_tuple (and the other heap_hot_indexed_* symbols) in src/backend/access/heap/*.c returns no matches. The breakpoints are therefore already stale/incorrect.

This belongs in the developer's local-only ignore (shell.nix already sets core.excludesFile .local-gitignore), not committed to the tree.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 prstate->processed[offnum] = true; inside heap_prune_record_dead(). Line-number breakpoints drift on any edit and will silently break on the wrong code. This whole file cross-contaminates unrelated WIP and should not be committed.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These line-number breakpoints are fragile and already stale. heap_hot_indexed_* symbols (referenced below as symbol breakpoints) do not exist anywhere under src/backend/access/heap/, and the hardcoded line numbers in heapam.c, heapam_indexscan.c, indexam.c, and pruneheap.c drift the moment those files change. heapam_indexscan.c is not even part of this change set. More fundamentally, this is personal debugger scaffolding: PostgreSQL's own root .gitignore states such local-workflow files "should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude" rather than committed. This file should not be in the repository.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 heap_hot_indexed_* functions (e.g. heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, heap_hot_indexed_read_bitmap, ...) exist anywhere in src/backend/access/heap, and heap_xlog_indexed_update is likewise absent. The hardcoded file:line breakpoints (heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299, heapam_indexscan.c:182/250/297, execIndexing.c:370, ...) point into the as-yet-unmerged HOT-indexed-updates series; heapam_indexscan.c is only 588 lines so heapam_indexscan.c:297 is live code but not the referenced logic. This is dead/aspirational scaffolding that will silently break on any edit. Remove from the patch. [high confidence]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded source line-number breakpoints (e.g. heapam.c:3210, heapam.c:4019, pruneheap.c:1802, indexam.c:299) are inherently fragile: they silently rebind to whatever code happens to occupy those lines after any edit. Combined with the fact that the targeted patch series is not in this tree, these line numbers are stale and unverifiable on day one. If this file is kept at all (it should not be committed), prefer function-name breakpoints over line numbers.

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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:

  1. The file:line breakpoints (break heapam.c:3210, heapam.c:4019, pruneheap.c:2936, indexam.c:299, etc.) are extremely fragile absolute source locations. They will silently point at the wrong code after any edit to those files and cannot be trusted to match the actual feature code.
  2. The symbol breakpoints reference functions that do not exist anywhere in this change -- heap_hot_indexed_create_tuple, heap_hot_search_buffer, and heap_xlog_indexed_update return no matches in the tree, so break will silently fail to resolve them. The referenced files (heapam.c, pruneheap.c, heapam_indexscan.c, heapam_xlog.c, indexam.c, execIndexing.c) are not even among the changed files in this series, which are planner/index-AM files. Remove this file from the patch.

[high confidence]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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: heap_hot_indexed_create_tuple, heap_hot_search_buffer, heap_xlog_indexed_update have no definitions) and a source file heapam_indexscan.c plus WAL record XLOG_HEAP2_INDEXED_UPDATE that are not in this review group. Hardcoded line/symbol breakpoints do not belong in a committed patch.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These line-number breakpoints are already wrong against this tree. heapam.c:3210 is the closing brace of heap_delete(), not the heap_update entry; heapam.c:4019 is a RelationGetBufferForTuple() argument, not the "pure HOT" decision. Line-number breakpoints are inherently fragile — any edit silently points them at unrelated code, making the aid actively misleading. Besides being out of scope, these must not be committed.


# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, etc. in the heap sources). They belong to an unmerged patch series not present in this change, so GDB will fail to set them. This confirms the file is aspirational scaffolding for work that has not landed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 heap_hot_indexed_create_tuple, heap_hot_indexed_tuple_size, heap_hot_indexed_serialize_bitmap, heap_hot_indexed_* helpers, heap_xlog_indexed_update, or the HEAP_INDEXED_UPDATED flag; pruneheap.c contains the standard pruning code, not the "redirect-with-data"/"collapsed redirect" logic described here. GDB will fail to set these breakpoints ("Function ... not defined"). This .gdbinit describes a "HOT indexed updates" patch series that is not present in this change set -- it references code that was never added, so it is dead scaffolding.

Comment on lines +42 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These symbol-based breakpoints reference HOT-indexed helper functions (heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, etc.) that do not exist in the tree. A search of src/backend/access/heap/ for heap_hot_indexed returns no matches, so GDB will fail to set these breakpoints. This confirms the file is tied to a different/divergent patch state and is misleading as committed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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: heap_hot_indexed_tuple_size, heap_hot_indexed_create_tuple, heap_hot_search_buffer, and heap_xlog_indexed_update all return no matches under src/backend/access/**. As committed, this file documents and binds to code that does not exist, so the breakpoints are dead/misleading. This is personal developer scaffolding that does not belong in a patch destined for pgsql-hackers/commitfest (violates the minimal-diff rule); it should be git-ignored, not committed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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, etc.). They belong to a HOT indexed-updates patch that is not present here, so GDB will fail to set the breakpoints. This confirms the file is stale, unverifiable scaffold that should not be in the repo.


# Create augmented tuple with embedded modified-column bitmap
break heap_hot_indexed_create_tuple
Comment on lines +45 to +46

ghost Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 heap_hot_indexed_create_tuple and heap_xlog_indexed_update do not exist anywhere in the source, so these symbol breakpoints will fail to resolve. The file is misleading and unmaintainable as committed.

Comment on lines +45 to +46

ghost Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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: heap_hot_indexed_create_tuple, heap_hot_indexed_serialize_bitmap, heap_hot_search_buffer, heap_xlog_indexed_update, log_heap_update with indexed_update handling, plus macros HEAP_INDEXED_UPDATED/XLOG_HEAP2_INDEXED_UPDATE referenced in comments. I searched .c/.h and found no matches. GDB will silently fail to set these (Function "..." not defined), making the file a broken/aspirational artifact. Combined with the hardcoded line numbers below (heapam.c:3210, heapam.c:4019, heapam_indexscan.c:182, pruneheap.c:1802, indexam.c:299, execIndexing.c:370), which drift to the wrong code on any edit, this debugging aid is actively misleading.

Comment on lines +45 to +46

ghost Oct 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

ghost Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

ghost Nov 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heap_xlog_indexed_update (and the XLOG_HEAP2_INDEXED_UPDATE macro named in the comment above) do not exist anywhere in the tree. The WAL-replay breakpoint cannot resolve, reinforcing that this .gdbinit targets code that is not part of this change.


# =========================================================================
# 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
18 changes: 18 additions & 0 deletions .github/workflows/fork-ocr-model-check.yml
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 }}
43 changes: 43 additions & 0 deletions .github/workflows/fork-ocr-review.yml
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 }}
48 changes: 48 additions & 0 deletions .github/workflows/fork-sync-upstream.yml
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
34 changes: 34 additions & 0 deletions contrib/btree_gist/expected/timestamp.out
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,37 @@ SELECT a, a <-> '2004-10-26 08:55:08' FROM timestamptmp ORDER BY a <-> '2004-10-
Mon Nov 29 20:12:43 2004 | @ 34 days 11 hours 17 mins 35 secs
(3 rows)

-- 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;
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------
Limit
Output: ((p <-> '(3,4)'::point)), ((a <-> 'Tue Oct 26 08:55:08 2004'::timestamp without time zone))
-> Index Scan using timestamppt_idx on public.timestamppt
Output: ((p <-> '(3,4)'::point)), (a <-> 'Tue Oct 26 08:55:08 2004'::timestamp without time zone)
Order By: ((timestamppt.p <-> '(3,4)'::point) AND (timestamppt.a <-> 'Tue Oct 26 08:55:08 2004'::timestamp without time zone))
Order By Values Used: 1
(6 rows)

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;
d | da
---+---------------------------
0 | @ 1 day 12 hours 23 mins
0 | @ 3 days 11 hours 52 mins
0 | @ 5 days 11 hours 21 mins
(3 rows)

RESET enable_indexonlyscan;
DROP TABLE timestamppt;
18 changes: 18 additions & 0 deletions contrib/btree_gist/sql/timestamp.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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

ghost Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final query orders only by p <-> point(3,4) then a <-> '2004-10-26 08:55:08', with no deterministic tie-breaker, and all three output rows share distance 0 on the primary key. Ordering therefore rests entirely on the interval distance. If two rows in the p=(3,4) group were equidistant from the reference timestamp (the static data has timestamps on both sides of it), or shared an interval value, LIMIT 3 row selection/order would be non-deterministic and the test could flake on the buildfarm across platforms. It passes for this exact data file because the three smallest distances happen to be distinct, but consider adding an explicit tie-breaker (e.g. a third ORDER BY column) to make the stability independent of the data. (low confidence)

FROM timestamppt
ORDER BY p <-> point(3, 4), a <-> '2004-10-26 08:55:08' LIMIT 3;
RESET enable_indexonlyscan;
DROP TABLE timestamppt;
Loading
Loading