fix(sql): segfault when using `touch()` on a table with a new empty column by nwoolmer · Pull Request #6561 · questdb/questdb

nwoolmer · 2025-12-18T18:13:49Z

When a new column is added to a table, it is initially empty. When you call frame.getPageAddress to get the column's base memory address, this empty column is uninitialised, and therefore returns an address of 0.

Unfortunately, touch() did not check this possibility, so would use the nullptr.

Additionally added checks for indexes. We do not have a repro for this, and theoretically the prior issue should hit before the index is tried, but makes sense to add these for safety.

coderabbitai · 2025-12-18T18:13:56Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Added defensive null-checks in the touch table function to skip processing empty memory regions with zero base addresses, preventing segmentation faults when cached query plans are reused after table schema modifications. Includes a corresponding test case validating the fix.

Changes

Cohort / File(s)	Change Summary
Core fix for touch() segfault prevention `core/src/main/java/io/questdb/griffin/engine/functions/table/TouchTableFunctionFactory.java`	Added guards to skip processing columns and indices when base addresses are zero (empty memory regions). Introduced logging for empty column data, index keys, and index values. Expanded imports to include PageFrame, PageFrameCursor, RecordMetadata, and SymbolTableSource types.
Test coverage for new column addition `core/src/test/java/io/questdb/test/griffin/engine/functions/TouchTableFunctionTest.java`	Added test method `testNoSegfaultWhenAddingNewColumns()` to verify that adding a new column to a WAL-enabled table does not cause segfaults during subsequent touch queries. Test creates table, runs touch query, adds column, re-runs touch queries, updates data, and verifies touch result changes appropriately.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Verify that checking for zero base addresses is a sufficient and complete fix for the segfault scenario
Confirm the defensive guards cover all memory access paths in the touch operation (columns, indexed columns, keys, values)
Validate that the test case accurately reproduces the original segfault conditions and validates the fix
Ensure logging statements do not introduce performance overhead for normal operations
Check whether similar null-check patterns should be applied elsewhere in the codebase

Suggested reviewers

bluestreak01
RaphDal

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main fix: addressing a segfault when using touch() on tables with new empty columns.
Linked Issues check	✅ Passed	The PR successfully addresses issue #6560 by adding guards to skip empty memory regions and logging for empty column data, preventing segfaults during touch() execution on tables with new columns.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the segfault issue: guards for empty regions in TouchTableFunctionFactory, supporting imports, and a comprehensive test case.
Description check	✅ Passed	The pull request description clearly relates to the changeset by explaining the bug being fixed and the specific changes made to prevent segfaults when using touch() on tables with new empty columns.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

nwoolmer · 2025-12-18T18:15:00Z

@coderabbitai review

coderabbitai · 2025-12-18T18:15:07Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

core/src/main/java/io/questdb/griffin/engine/functions/table/TouchTableFunctionFactory.java (1)

173-177: Critical segfault fix - null-check prevents crash.

This null-check correctly addresses the reported segfault that occurred when touch() was called after adding an empty column to a table. The defensive guard prevents Unsafe.getByte from accessing invalid memory at address 0.

Consider using LOG.debug() instead of LOG.info() if empty columns are a common scenario in normal operations, to reduce log verbosity.

core/src/test/java/io/questdb/test/griffin/engine/functions/TouchTableFunctionTest.java (1)

45-81: Test correctly validates the segfault fix.

This test reproduces the exact crash scenario from issue #6560: creating a table, running touch(), altering to add a column, then running touch() again. The test validates:

No segfault occurs when touching after ALTER TABLE ADD COLUMN

Touch results remain consistent when the new column is empty (lines 73-74)

Touch results update correctly after populating the new column (line 79)

Consider adding a test case for indexed columns with empty data to validate the index-specific null-checks at lines 187-190 and 196-199 of TouchTableFunctionFactory.java.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 861e006 and a8c334b.

📒 Files selected for processing (2)

core/src/main/java/io/questdb/griffin/engine/functions/table/TouchTableFunctionFactory.java (2 hunks)
core/src/test/java/io/questdb/test/griffin/engine/functions/TouchTableFunctionTest.java (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (36)

GitHub Check: New pull request (SelfHosted Running tests with cover on linux-other)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-pgwire)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-cairo-sub)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-cairo-root)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-fuzz2)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-fuzz1)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-griffin-sub)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-griffin-root)
GitHub Check: New pull request (Rust Test and Lint on linux-jdk17)
GitHub Check: New pull request (Trigger Enterprise CI Trigger Enterprise Pipeline)
GitHub Check: New pull request (SelfHosted Other tests on linux-x64-zfs)
GitHub Check: New pull request (SelfHosted Other tests on linux-x86-graal)
GitHub Check: New pull request (SelfHosted Griffin tests on linux-x86-graal)
GitHub Check: New pull request (SelfHosted Other tests on linux-arm64)
GitHub Check: New pull request (SelfHosted Cairo tests on linux-x64-zfs)
GitHub Check: New pull request (SelfHosted Cairo tests on linux-x86-graal)
GitHub Check: New pull request (SelfHosted Griffin tests on linux-x64-zfs)
GitHub Check: New pull request (SelfHosted Cairo tests on linux-arm64)
GitHub Check: New pull request (SelfHosted Griffin tests on linux-arm64)
GitHub Check: New pull request (Hosted Running tests on windows-other-2)
GitHub Check: New pull request (Hosted Running tests on windows-other-1)
GitHub Check: New pull request (Hosted Running tests on windows-pgwire)
GitHub Check: New pull request (Hosted Running tests on windows-cairo-2)
GitHub Check: New pull request (Hosted Running tests on windows-cairo-1)
GitHub Check: New pull request (Hosted Running tests on windows-fuzz2)
GitHub Check: New pull request (Hosted Running tests on windows-fuzz1)
GitHub Check: New pull request (Hosted Running tests on windows-griffin-sub)
GitHub Check: New pull request (Hosted Running tests on windows-griffin-base)
GitHub Check: New pull request (Hosted Running tests on mac-other)
GitHub Check: New pull request (Hosted Running tests on mac-pgwire)
GitHub Check: New pull request (Hosted Running tests on mac-cairo-fuzz)
GitHub Check: New pull request (Hosted Running tests on mac-cairo)
GitHub Check: New pull request (Hosted Running tests on mac-griffin)
GitHub Check: New pull request (Check Changes Check changes)
GitHub Check: build
GitHub Check: build

🔇 Additional comments (2)

core/src/main/java/io/questdb/griffin/engine/functions/table/TouchTableFunctionFactory.java (2)

187-190: Indexed column key protection - consistent with data column check.

The null-check for empty index key memory is correctly placed and prevents segfaults when touching indexed columns with no key data.

196-199: Indexed column value protection completes the defensive coverage.

This third null-check ensures all memory access points in the touch operation are protected from null pointer dereferences. The fix comprehensively addresses the segfault risk.

- Single point of validation instead of 3 separate checks - Remove noisy INFO-level logging for normal operation - All callers automatically protected 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

glasstiger · 2025-12-18T20:05:03Z

[PR Coverage check]

😍 pass : 2 / 2 (100.00%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/griffin/engine/functions/table/TouchTableFunctionFactory.java	2	2	100.00%

fix segfault in touch after schema change

a8c334b

nwoolmer added Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution labels Dec 18, 2025

coderabbitai bot reviewed Dec 18, 2025

View reviewed changes

nwoolmer and others added 2 commits December 18, 2025 18:52

mismatched page sizes between mac (16K) and linux (4K)

79a3759

bluestreak01 approved these changes Dec 18, 2025

View reviewed changes

bluestreak01 merged commit 944b63d into master Dec 18, 2025
43 checks passed

bluestreak01 deleted the nw_touch branch December 18, 2025 23:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sql): segfault when using `touch()` on a table with a new empty column#6561

fix(sql): segfault when using `touch()` on a table with a new empty column#6561
bluestreak01 merged 3 commits intomasterfrom
nw_touch

nwoolmer commented Dec 18, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Dec 18, 2025 •

edited

Loading

Review skipped

Uh oh!

nwoolmer commented Dec 18, 2025

Uh oh!

coderabbitai bot commented Dec 18, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

glasstiger commented Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nwoolmer commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

nwoolmer commented Dec 18, 2025

Uh oh!

coderabbitai bot commented Dec 18, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

glasstiger commented Dec 18, 2025

[PR Coverage check]

file detail

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nwoolmer commented Dec 18, 2025 •

edited

Loading

coderabbitai bot commented Dec 18, 2025 •

edited

Loading