Skip to content

fix(sql): support for decimal types in group by fill and selected cursor#6532

Merged
bluestreak01 merged 2 commits intomasterfrom
rd_fix_selected_decimal
Dec 13, 2025
Merged

fix(sql): support for decimal types in group by fill and selected cursor#6532
bluestreak01 merged 2 commits intomasterfrom
rd_fix_selected_decimal

Conversation

@RaphDal
Copy link
Copy Markdown
Contributor

@RaphDal RaphDal commented Dec 13, 2025

Fixes #6530

Summary

Using SAMPLE BY ... FILL(NULL) on a table containing decimal columns and then filtering for NULL values in the outer query throws UnsupportedOperationException: Record.getDecimal128().

The issue occurs because FillRangeRecordCursorFactory.FillRangeRecord and SelectedRecord classes were missing implementations for the getDecimal*() methods, causing them to fall through to the default Record interface methods that throw UnsupportedOperationException.

This PR adds support for all decimal types in:

  • FillRangeRecordCursorFactory.FillRangeRecord - handles gap filling with null values for decimal columns
  • SelectedRecord - properly delegates decimal column access to the base record using the correct column index mapping

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 13, 2025 •
edited
Loading

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

The changes add support for decimal data types (Decimal128, Decimal256, and fixed-size decimals) in two record accessor classes to enable gap-filling functionality for decimal columns in sample-by queries. A corresponding test case validates this behavior.

Changes

Cohort / File(s) Summary
Decimal type accessor methods
core/src/main/java/io/questdb/griffin/engine/groupby/FillRangeRecordCursorFactory.java, core/src/main/java/io/questdb/griffin/engine/table/SelectedRecord.java		 Added six decimal accessor method overrides: getDecimal128(), getDecimal256(), getDecimal64(), getDecimal32(), getDecimal16(), getDecimal8(). Each method delegates to the fill function or base record based on gap-filling state. Imports for Decimal128 and Decimal256 added.
Gap-fill decimal test
core/src/test/java/io/questdb/test/griffin/engine/groupby/SampleByTest.java		 Added testSampleByFillNullDecimal() test method verifying sample-by with fill(null) behavior on decimal columns, including null filling, previous values, interval offsets, and calendar alignment scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Method implementation consistency: Verify that decimal accessor delegation logic in both classes mirrors existing non-decimal patterns and correctly handles gap-filling state
  • Test coverage: Review comprehensiveness of testSampleByFillNullDecimal() test cases, particularly edge cases with null decimals and various fill strategies
  • Type handling: Confirm proper handling of all six decimal variants, especially Decimal128 and Decimal256 sink parameters

Suggested labels

Bug, SQL

Suggested reviewers

  • bluestreak01
  • puzpuzpuz

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 change: adding decimal type support to group by fill and selected cursor operations.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the issue, root cause, and which classes were modified to fix the problem.
Linked Issues check ✅ Passed The PR successfully addresses issue #6530 by implementing getDecimal*() methods in FillRangeRecord and SelectedRecord to support decimal column access during SAMPLE BY...FILL(NULL) operations with null value filtering.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the decimal type support issue: modifications to two classes and one test method to verify the fix, with no unrelated changes.

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 Dec 13, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 13, 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.

@glasstiger
Copy link
Copy Markdown
Contributor

glasstiger commented Dec 13, 2025

[PR Coverage check]

😍 pass : 28 / 28 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/engine/groupby/FillRangeRecordCursorFactory.java 20 20 100.00%
🔵 io/questdb/griffin/engine/table/SelectedRecord.java 8 8 100.00%

@bluestreak01 bluestreak01 merged commit c0b4ace into master Dec 13, 2025
43 checks passed
@bluestreak01 bluestreak01 deleted the rd_fix_selected_decimal branch December 13, 2025 23:03
mtopolnik pushed a commit that referenced this pull request Dec 16, 2025
@RaphDal @mtopolnik
fix(sql): support for decimal types in group by fill and selected cur…
a02faa7 
…sor (#6532)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluestreak01 bluestreak01 bluestreak01 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Null value in a decimal field after 'sample by and fill' leads to UnsupportedOperationException

3 participants

@RaphDal @glasstiger @bluestreak01