-
Notifications
You must be signed in to change notification settings - Fork 1
Tepid: only update indexes that have changed attributes #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
68556b4
e90dbdc
4d59990
373b4af
8569cb7
691317f
7e96de2
a40f098
5c24898
2a4e763
32ddfea
f439210
322c0cc
64a2a88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| Diagnostics: | ||
| MissingIncludes: None | ||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Beyond not belonging in the tree, the contents are also non-portable and stale: the hardcoded relative include path |
||
| InlayHints: | ||
| Enabled: true | ||
| ParameterNames: true | ||
| DeducedTypes: true | ||
| CompileFlags: | ||
| CompilationDatabase: build/ # Search build/ directory for compile_commands.json | ||
| 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| # HOT Indexed Updates — GDB breakpoints for code review | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file is a personal developer debugging artifact and must not be committed to a PostgreSQL patch destined for pgsql-hackers/commitfest. It is unrelated to the feature implementation and violates the 'minimal diff' discipline -- one of the top rejection reasons on -hackers. It is added alongside other stray developer-only files in this same change ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||
| # | ||
| # 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-ASCII em-dash characters (U+2014) appear in the comment headers throughout this file (lines 1, 15, 38, 52, 81, 98, 106, 117, 140, 148). PostgreSQL requires ASCII-only content in source and diffs; em-dashes, smart quotes and ellipses are disallowed. If any of this were retained, replace the em-dashes with a plain ASCII hyphen/dash. Confidence: high. |
||
| # src/backend/access/heap/heapam.c | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-ASCII punctuation (em-dash U+2014) in comment text. PostgreSQL source and diffs must be ASCII-only; em/en-dashes and smart quotes are not permitted. This occurs throughout the header comments (lines 1, 15, 38, 52, 81, 98, 106, 117, 140, 148). Even setting aside that the file should be removed, this would fail the ASCII-only rule if reused in real source. |
||
| # ========================================================================= | ||
|
|
||
| # Main entry: heap_update | ||
| break heapam.c:3210 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Breakpoints pinned to hard-coded line numbers in files modified by this very patch series (heapam.c:3210/4019/4024/4033/4101/4147, heapam_indexscan.c:182/250/297, indexam.c:299, execIndexing.c:370, pruneheap.c:1287/1802/1836/1863/2936) are extremely fragile. Any further edit shifts every subsequent line, so the breakpoints silently land on the wrong statements and the annotations (e.g. 'Line 4019: pure HOT', 'Line 4024: HOT indexed path') cannot be trusted. Another reason this file should not be tracked in the repo. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Breakpoints set on hard-coded absolute source line numbers are extremely brittle: any edit to these files (or a rebase) immediately makes them point at the wrong statement, silently misleading anyone who sources the file. The accompanying comments (e.g. 'Line 4019: pure HOT') hardcode line placement that will drift. This applies to every |
||
|
|
||
| # 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 | ||
|
Comment on lines
+27
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hard-coded file:line breakpoints (heapam.c:3210/4019/4024/4033/4101/4147, heapam_indexscan.c:182/250/297, indexam.c:299, execIndexing.c:370, pruneheap.c:1287/1802/1836/1863/2936) are inherently fragile: they silently drift to the wrong statement whenever the source is edited — including by this very patch series. The accompanying comments assert exact per-line semantics (e.g. "Line 4019: pure HOT", "Line 4147: Restore HEAD_INDEXED_UPDATED on old tuple"), which become stale and misleading the moment the code moves. If any GDB helper were kept at all, it should use function-name breakpoints only. Confidence: high. |
||
|
|
||
| # Set HEAP_INDEXED_UPDATED flag on new tuple before page insertion | ||
| break heapam.c:4101 | ||
|
|
||
| # Restore HEAP_INDEXED_UPDATED on old tuple (only if it previously had it) | ||
| break heapam.c:4147 | ||
|
|
||
| # ========================================================================= | ||
| # 2. TUPLE CREATION — building the augmented tuple with embedded bitmap | ||
| # src/backend/access/heap/heapam.c | ||
| # ========================================================================= | ||
|
|
||
| # Predict augmented tuple size (returns 0 if t_hoff would overflow) | ||
| break heap_hot_indexed_tuple_size | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These function-name breakpoints reference symbols that do not exist in the patched source. The tuple-building code is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function-name breakpoint on a symbol that does not exist. A codebase search finds no |
||
|
|
||
| # Create augmented tuple with embedded modified-column bitmap | ||
| break heap_hot_indexed_create_tuple | ||
|
|
||
| # 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 | ||
|
|
||
| # Accumulate bitmap from INDEXED_UPDATED tuple in chain | ||
| break heapam_indexscan.c:250 | ||
|
|
||
| # Stale entry detection: accumulated bitmap overlaps this index's attrs | ||
| break heapam_indexscan.c:297 | ||
|
|
||
| # ========================================================================= | ||
| # 5. INDEX SCAN SETUP — indexed_attrs bitmap computation | ||
| # src/backend/access/index/indexam.c | ||
| # ========================================================================= | ||
|
|
||
| # Compute indexed_attrs for HOT indexed update chain following | ||
| break indexam.c:299 | ||
|
|
||
| # ========================================================================= | ||
| # 6. INDEX INSERTION — skip unchanged indexes for HOT indexed updates | ||
| # src/backend/executor/execIndexing.c | ||
| # ========================================================================= | ||
|
|
||
| # Entry: insert/update index tuples | ||
| break ExecInsertIndexTuples | ||
|
|
||
| # Index skip decision: skip indexes whose attrs don't overlap modified set | ||
| break execIndexing.c:370 | ||
|
|
||
| # ========================================================================= | ||
| # 7. PRUNING — chain collapsing and redirect-with-data | ||
| # src/backend/access/heap/pruneheap.c | ||
| # ========================================================================= | ||
|
|
||
| # Main prune function | ||
| break heap_page_prune_and_freeze | ||
|
|
||
| # Per-chain pruning entry | ||
| break heap_prune_chain | ||
|
|
||
| # Chain collapsing: collect bitmaps from dead INDEXED_UPDATED intermediates | ||
| break pruneheap.c:1802 | ||
|
|
||
| # OR dead tuple bitmaps into combined bitmap | ||
| break pruneheap.c:1836 | ||
|
|
||
| # Record redirect-with-data for execute phase | ||
| break pruneheap.c:1863 | ||
|
|
||
| # Execute phase: apply redirect-with-data entries on the page | ||
| break pruneheap.c:1287 | ||
|
|
||
| # ========================================================================= | ||
| # 8. WAL REPLAY — recovery of HOT indexed updates | ||
| # src/backend/access/heap/heapam_xlog.c | ||
| # ========================================================================= | ||
|
|
||
| # WAL replay for XLOG_HEAP2_INDEXED_UPDATE | ||
| break heap_xlog_indexed_update | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function-name breakpoint on a nonexistent symbol: a codebase search finds no |
||
|
|
||
| # ========================================================================= | ||
| # 9. WAL LOGGING — writing HOT indexed update records | ||
| # src/backend/access/heap/heapam.c | ||
| # ========================================================================= | ||
|
|
||
| # WAL logging for heap updates (handles indexed_update flag) | ||
| break log_heap_update | ||
|
|
||
| # Serialize redirect-with-data into WAL record (pruneheap.c) | ||
| break pruneheap.c:2936 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Node modules | ||
| scripts/ai-review/node_modules/ | ||
| # Note: package-lock.json should be committed for reproducible CI/CD builds | ||
|
|
||
| # Logs | ||
| scripts/ai-review/cost-log-*.json | ||
| scripts/ai-review/*.log | ||
|
|
||
| # OS files | ||
| .DS_Store | ||
| Thumbs.db | ||
|
|
||
| # Editor files | ||
| *.swp | ||
| *.swo | ||
| *~ | ||
| .vscode/ | ||
| .idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
.clangdfile is a personal editor/LSP configuration and should not be committed to the tree. The repository's own.gitignore(lines 1-3) explicitly states the project convention: "Auxiliary files from local workflows, your preferred editor, etc. should be ignored locally using $GIT_DIR/info/exclude or ~/.gitexclude." A patch destined for pgsql-hackers/commitfest must not add personal tooling config; committing it violates the minimal-diff rule and would be an immediate rejection reason. Keep this file local (via $GIT_DIR/info/exclude) rather than adding it to the repo.