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.

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.

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

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.

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

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

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.

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


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


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

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

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.


# 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

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

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

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