Skip to content

fix: colDataKeepFirstNRows reset varmeta.length when all kept rows ar…#35061

Merged
guanshengliang merged 2 commits intomainfrom
fix/uFatal-log-level
Apr 7, 2026
Merged

fix: colDataKeepFirstNRows reset varmeta.length when all kept rows ar…#35061
guanshengliang merged 2 commits intomainfrom
fix/uFatal-log-level

Conversation

@facetosea
Copy link
Copy Markdown
Contributor

…e NULL

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

@facetosea facetosea requested review from a team, dapan1121 and guanshengliang as code owners April 3, 2026 09:35
Copilot AI review requested due to automatic review settings April 3, 2026 09:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a variable-length column trimming edge case in blockDataKeepFirstNRows/colDataKeepFirstNRows where keeping only NULL rows could leave varmeta.length stale, and adds a regression test to cover LIMIT/OFFSET scenarios that exercise that path.

Changes:

  • Reset SColumnInfoData::varmeta.length to 0 when the kept range contains no non-NULL var-length values.
  • Add a Python system test covering LIMIT and LIMIT+OFFSET on tables where the first N var-length values are all NULL.
  • Clarify/adjust numeric_limits<intx::uint<N>>::digits10 computation documentation and constant precision.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
source/common/src/tdatablock.c Resets varmeta.length to 0 when all kept var-length rows are NULL to avoid stale length usage after LIMIT trimming.
test/cases/01-DataTypes/test_null_column.py Adds regression coverage for LIMIT/LIMIT+OFFSET on var-length columns with leading NULLs.
source/libs/decimal/src/detail/intx/int128.hpp Documents digits10 derivation and uses a more precise log10(2) constant in the constexpr calculation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses an issue where variable-length columns could retain stale length values when all rows in a data block are NULL following a LIMIT operation. It ensures varmeta.length is reset to 0 in such cases and replaces a fatal error with a debug log. Additionally, the precision of the digits10 constant in the decimal library was improved, and a new regression test was added. Review feedback suggests refining the condition check for NULL offsets and verifying format specifiers in logging to prevent potential sign-mismatch issues.

Comment thread source/common/src/tdatablock.c Outdated
Copy link
Copy Markdown
Contributor

@dapan1121 dapan1121 left a comment

Choose a reason for hiding this comment

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

LGTM

@guanshengliang guanshengliang merged commit 0c7c05d into main Apr 7, 2026
12 of 16 checks passed
@facetosea facetosea deleted the fix/uFatal-log-level branch April 20, 2026 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants