Skip to content

fix(core): fix a bug where default ILP decimal wouldn't be null#6454

Merged
bluestreak01 merged 3 commits intomasterfrom
rd_fix_decimal_null_ilp
Nov 28, 2025
Merged

fix(core): fix a bug where default ILP decimal wouldn't be null#6454
bluestreak01 merged 3 commits intomasterfrom
rd_fix_decimal_null_ilp

Conversation

@RaphDal
Copy link
Copy Markdown
Contributor

@RaphDal RaphDal commented Nov 25, 2025

This pull request updates how null/default values for decimal types are handled in both TableWriter and WalWriter, switching from hardcoded primitive constants to centralized constants in the Decimals class. It also adds a new test to ensure that decimal columns default to null values when not provided. These changes improve consistency, maintainability, and correctness in handling decimal nulls.

Decimal null value handling improvements:

  • Replaced hardcoded primitive constants (e.g., Byte.MIN_VALUE, Short.MIN_VALUE, etc.) with Decimals class constants (e.g., Decimals.DECIMAL8_NULL, Decimals.DECIMAL16_NULL, etc.) for all decimal types in the configureNullSetters methods of both TableWriter and WalWriter. This ensures a single source of truth for decimal null representations and reduces the risk of errors if null values change in the future. [1] [2]
  • Added imports for the Decimals class in both TableWriter.java and WalWriter.java to support the above change. [1] [2]

Testing improvements:

  • Added a new test testDecimalDefaultValues in LineHttpSenderTest.java to verify that decimal columns are set to their default null values when not explicitly provided in the input. This test creates a table with various decimal columns, inserts a row without specifying decimal values, and asserts that the resulting row has nulls in all decimal columns.

@RaphDal RaphDal requested a review from bluestreak01 November 25, 2025 21:12
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 25, 2025

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

Standardizes NULL value representation for decimal columns by replacing primitive MIN_VALUE sentinels with dedicated Decimals constants in TableWriter and WalWriter classes. A test method validates decimal column default value behavior in line protocol inserts.

Changes

Cohort / File(s) Change Summary
Decimal null-value standardization
core/src/main/java/io/questdb/cairo/TableWriter.java, core/src/main/java/io/questdb/cairo/wal/WalWriter.java
Replaced primitive MIN_VALUE sentinels with dedicated Decimals constants for DECIMAL8, DECIMAL16, DECIMAL32, DECIMAL64, DECIMAL128, and DECIMAL256 types in configureNullSetters methods. Added Decimals import to both classes.
Test for decimal default values
core/src/test/java/io/questdb/test/cutlass/http/line/LineHttpSenderTest.java
Added testDecimalDefaultValues() to verify that DECIMAL columns retain default (empty) values when not explicitly supplied in line protocol inserts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify all six decimal types (DECIMAL8, DECIMAL16, DECIMAL32, DECIMAL64, DECIMAL128, DECIMAL256) are correctly mapped to their corresponding Decimals constants in both TableWriter and WalWriter
  • Confirm the test exercises all affected decimal types
  • Ensure consistency of null-value behavior between the two writer implementations

Suggested labels

Bug, ILP

Suggested reviewers

  • bluestreak01

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 (2 passed)
Check name Status Explanation
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose of replacing hardcoded primitive constants with Decimals class constants and the addition of a test for verification.
Title check ✅ Passed The title 'fix(core): fix a bug where default ILP decimal wouldn't be null' clearly summarizes the main change: fixing decimal null value handling in ILP (Inline Line Protocol) by replacing hardcoded primitives with centralized Decimals constants.

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.

@RaphDal
Copy link
Copy Markdown
Contributor Author

RaphDal commented Nov 25, 2025

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 25, 2025

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

@RaphDal RaphDal changed the title fix(core): fix invalid decimal null values coming from ILP fix(core): fix a bug where default ILP decimal wouldn't be null Nov 25, 2025
@bluestreak01
Copy link
Copy Markdown
Member

@RaphDal please submit Ent tandem PR

@bluestreak01
Copy link
Copy Markdown
Member

tandem should pass CI

@glasstiger
Copy link
Copy Markdown
Contributor

[PR Coverage check]

😍 pass : 12 / 12 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/cairo/TableWriter.java 6 6 100.00%
🔵 io/questdb/cairo/wal/WalWriter.java 6 6 100.00%

@bluestreak01 bluestreak01 merged commit 8a85eb3 into master Nov 28, 2025
43 checks passed
@bluestreak01 bluestreak01 deleted the rd_fix_decimal_null_ilp branch November 28, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants