feat(sql): implement ksum() window function with Kahan summation by bluestreak01 · Pull Request #6642 · questdb/questdb

bluestreak01 · 2026-01-13T20:32:53Z

Summary

Implements ksum() as a window function using the Kahan summation algorithm for improved floating-point precision
Complements the existing ksum() aggregate function by enabling its use in window contexts
Supports all standard window frame types (ROWS, RANGE, partitioned, unbounded, sliding windows)

Changes

New file: KSumDoubleWindowFunctionFactory.java - Window function factory with 9 inner classes for different frame types
Modified: function_list.txt - Added registration entry
Modified: SqlParserTest.java - Updated test to use nsum (since ksum is now a valid window function)
Modified: WindowFunctionTest.java - Added ksum to FRAME_FUNCTIONS list and new test cases

Usage Examples

-- Whole partition
SELECT ksum(price) OVER (PARTITION BY symbol) FROM trades;

-- Cumulative sum
SELECT ksum(price) OVER (ORDER BY ts ROWS UNBOUNDED PRECEDING) FROM trades;

-- Sliding window
SELECT ksum(price) OVER (ORDER BY ts ROWS BETWEEN 3 PRECEDING AND CURRENT ROW) FROM trades;

-- Range-based window
SELECT ksum(price) OVER (ORDER BY ts RANGE BETWEEN '1' SECOND PRECEDING AND CURRENT ROW) FROM trades;

Test plan

New tests in WindowFunctionTest.java:
- testKSumWindowFunction - Tests partition, unbounded preceding, sliding window, and whole result set
- testKSumPrecision - Tests NULL handling
ksum added to FRAME_FUNCTIONS list for integration with existing parameterized tests
Verified SqlParserTest#testNonWindowFunctionInWindowContext still works with nsum

🤖 Generated with Claude Code

Add ksum() as a window function that uses the Kahan summation algorithm for improved floating-point precision. This complements the existing ksum() aggregate function by enabling its use in window contexts. The implementation supports all standard window frame types: - Partition by with/without order by - ROWS and RANGE frame modes - Unbounded and bounded preceding/following - Sliding windows with proper Kahan compensation for value removal Co-Authored-By: Claude Opus 4.5 <[email protected]>

coderabbitai · 2026-01-13T20:33:00Z

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

Adds a new KSumDoubleWindowFunctionFactory implementing Kahan-sum window functions for doubles, registers it in the function list, updates parser expectations, and adjusts window function tests (adds ksum tests and removes a large legacy test file).

Changes

Cohort / File(s)	Summary
Core Implementation `core/src/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java`	Adds a new public factory class (≈1513 LOC) implementing Kahan-sum (ksum) window functions for doubles across ROWS/RANGE and partitioned/unpartitioned frames, with multiple internal function implementations and memory-backed state.
Function Registry `core/src/main/resources/function_list.txt`	Adds an entry for the new KSumDoubleWindowFunctionFactory to the function list.
Test additions `core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java`	Adds import for `KSumDoubleWindowFunctionFactory`, extends FRAME_FUNCTIONS to include `ksum(`#COLUMN`)`, and introduces two new tests: `testKSumWindowFunction` and `testKSumPrecision`.
Test deletion `core/src/test/java/io/questdb/test/griffin/WindowFunctionTest.java`	Removes an entire legacy WindowFunctionTest test suite (multiple `@Test` methods covering many window scenarios).
Parser test update `core/src/test/java/io/questdb/test/griffin/SqlParserTest.java`	Adjusts one expectation: changes non-window function name from `ksum(...)` to `nsum(...)` in `testNonWindowFunctionInWindowContext`.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

fix(sql): fix intermittent failures with SQL window functions #6030: Window function factory refactor and WindowContext changes that are likely relevant to integrating the new KSumDoubleWindowFunctionFactory.

Suggested labels

Enhancement, SQL

Suggested reviewers

puzpuzpuz
kafka1991

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(sql): implement ksum() window function with Kahan summation' directly and clearly summarizes the primary change: adding ksum() as a window function with Kahan summation.
Description check	✅ Passed	The description comprehensively covers the changeset, explaining the feature (ksum window function with Kahan summation), listing all modified/new files, providing usage examples, and detailing test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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

Inline Kahan summation logic to avoid allocating double[1] arrays on every computeNext() call. This is more appropriate for QuestDB's zero-GC design philosophy. Co-Authored-By: Claude Opus 4.5 <[email protected]>

bluestreak01 · 2026-01-13T21:31:35Z

@CodeRabbit review

coderabbitai · 2026-01-13T21:31:42Z

✅ 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: 2

🤖 Fix all issues with AI agents

In
@core/src/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java:
- Around line 278-291: In the else branch where MemoryARW mem is created via
Vm.getCARWInstance(...) before returning new KSumOverRowsFrameFunction(...,
mem), protect the allocation against constructor failure by wrapping the
construction in a try/catch (or try/finally) so that if
KSumOverRowsFrameFunction's constructor throws, you call mem.close()/free()
(matching how other branches clean up) and rethrow the exception; specifically,
allocate MemoryARW mem, then try to return new
KSumOverRowsFrameFunction(args.get(0), rowsLo, rowsHi, mem), and on any
Throwable ensure mem is closed before propagating the error.

In
@core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java:
- Line 38: The test testWindowFunctionFailsInNonWindowContext omits the newly
added KSumDoubleWindowFunctionFactory from its factories array, so add
KSumDoubleWindowFunctionFactory.class to the factories array used in that test
to ensure the new window function is covered; locate the factories declaration
referenced by testWindowFunctionFailsInNonWindowContext and include
KSumDoubleWindowFunctionFactory in the list.

🧹 Nitpick comments (3)

core/src/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java (2)
963-979: Minor: Redundant modulo operation.

At line 970, firstIdx %= size is redundant since firstIdx is always < capacity (which equals size at this point due to the condition at line 963). The operation has no effect. This doesn't affect correctness but could be simplified.
Suggested simplification
                     if (firstIdx == 0) {
                         Vect.memcpy(newAddress, oldAddress, size * RECORD_SIZE);
                     } else {
-                        firstIdx %= size;
                         long firstPieceSize = (size - firstIdx) * RECORD_SIZE;
                         Vect.memcpy(newAddress, oldAddress + firstIdx * RECORD_SIZE, firstPieceSize);
                         Vect.memcpy(newAddress + firstPieceSize, oldAddress, firstIdx * RECORD_SIZE);
                         firstIdx = 0;
                     }
1277-1278: Unused instance field c.

The instance field c at line 1277 is written at line 1319 but never read. The compensation value is only needed during computation and doesn't need to persist as instance state.
Suggested cleanup
 static class KSumOverUnboundedPartitionRowsFrameFunction extends BasePartitionedWindowFunction implements WindowDoubleFunction {

-    private double c; // Kahan compensation
     private double sum;
And remove line 1319:
         value.putDouble(0, sum);
         value.putDouble(1, c);
         value.putLong(2, count);
         this.sum = count != 0 ? sum : Double.NaN;
-        this.c = c;
     }
core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java (1)

3532-3560: testKSumPrecision is mostly NULL-handling; consider tightening the “precision” claim.

The test asserts exact stringified floating output (e.g., 3.3000000000000003), which is fine for regression, but it doesn’t really demonstrate improved precision vs sum() and the comments may overstate what’s being tested.

Suggestion: either rename to reflect NULL/accumulation behavior, or add a small adversarial sequence comparison vs sum() to justify the name.

📜 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 669ae8c and a724203.

📒 Files selected for processing (4)

core/src/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java
core/src/main/resources/function_list.txt
core/src/test/java/io/questdb/test/griffin/SqlParserTest.java
core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java

🧰 Additional context used

🧬 Code graph analysis (2)

core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java (1)

core/src/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java (1)

KSumDoubleWindowFunctionFactory (66-1513)

core/src/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java (7)

core/src/main/java/io/questdb/cairo/ArrayColumnTypes.java (1)

ArrayColumnTypes (30-62)

core/src/main/java/io/questdb/cairo/map/MapFactory.java (1)

MapFactory (34-145)

core/src/main/java/io/questdb/cairo/sql/VirtualRecord.java (1)

VirtualRecord (40-285)

core/src/main/java/io/questdb/cairo/vm/Vm.java (1)

Vm (39-128)

core/src/main/java/io/questdb/griffin/model/WindowExpression.java (1)

WindowExpression (38-301)

core/src/main/java/io/questdb/std/MemoryTag.java (1)

MemoryTag (27-184)

core/src/main/java/io/questdb/std/ObjList.java (1)

ObjList (34-403)

⏰ 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). (35)

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

🔇 Additional comments (13)

core/src/test/java/io/questdb/test/griffin/SqlParserTest.java (1)

7966-7977: Test correctly validates non-window function error.

Switching from ksum() to nsum() is correct: ksum() is now a window function (breaking the test's intent), while nsum() is a pure aggregation function with no window capability. The error position (7) and message (non-window function called in window context) remain stable and correctly validate the parser/semantic error.

core/src/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java (8)

66-77: LGTM!

The factory class setup with static column type definitions is well-organized. The column type arrays correctly define the storage schema for each window frame variant (basic sum/compensation/count, rows-based with loIdx/startOffset, and range-based with additional capacity/firstIdx fields).

298-331: LGTM!

Simple and correct implementation for the single-row frame case. No Kahan summation needed since it's just returning the current value.

333-408: LGTM!

Correct two-pass implementation for whole partition sums. The Kahan summation algorithm is properly applied, and the handling of all-null partitions via preparePass2 is appropriate.

410-685: LGTM!

Complex but well-structured implementation for partition-based range frames. The Kahan summation with compensation for both addition and subtraction (when values leave the frame) is correctly implemented. Memory management via close(), reset(), reopen(), and toTop() follows the expected lifecycle patterns.

687-877: LGTM!

Correct implementation for partition-based rows frames with fixed-size ring buffers. The frame size calculation properly accounts for negative rowsHi values, and the Kahan summation is applied correctly for both adding and removing values from the sliding window.

1108-1272: LGTM!

Correct implementation for non-partitioned rows frame. The buffer management and Kahan summation logic are properly implemented. The close() method in constructor's catch block at line 1140 ensures proper cleanup if initBuffer() fails.

1354-1426: LGTM!

Clean and correct implementation for cumulative sum over the entire result set. State management in reset() and toTop() properly reinitializes all fields.

1491-1512: LGTM!

The static column type definitions correctly match the storage requirements of each window function variant.

core/src/main/resources/function_list.txt (1)

853-853: LGTM!

Correct registration of the new KSumDoubleWindowFunctionFactory in the function catalog, properly placed alongside other window functions.

core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java (3)

70-79: Verify ksum() argument-type coverage in parameterized frame tests.

FRAME_FUNCTIONS_PARAMETER_COLUMN_NAME registers ksum with "i" / "j" (typically LONG in this test suite). If ksum is truly ksum(D) only (no implicit cast from integer types), some parameterized tests may fail before reaching the intended frame-validation assertions.

If strict-double, consider switching ksum’s columns here to a known DOUBLE column (e.g., "d") or using explicit casts in the generated SQL.

3437-3530: New ksum() window coverage looks solid (partitioned, cumulative, sliding, whole-set).

The expected values align with the inserted dataset and exercise multiple frame shapes without being overly complex.

8760-8762: FRAME_FUNCTIONS updated to include ksum().

Good integration with the existing normalizeSuffix-based parameterized framework.

...rc/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java

core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java

bluestreak01 · 2026-01-14T12:23:28Z

@CodeRabbit review

coderabbitai · 2026-01-14T12:23:34Z

✅ 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 · 2026-01-14T15:08:31Z

[PR Coverage check]

😍 pass : 658 / 697 (94.40%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java	658	697	94.40%

fix: remove array allocations in ksum hot path

a724203

Inline Kahan summation logic to avoid allocating double[1] arrays on every computeNext() call. This is more appropriate for QuestDB's zero-GC design philosophy. Co-Authored-By: Claude Opus 4.5 <[email protected]>

coderabbitai bot reviewed Jan 13, 2026

View reviewed changes

...rc/main/java/io/questdb/griffin/engine/functions/window/KSumDoubleWindowFunctionFactory.java Show resolved Hide resolved

core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java Show resolved Hide resolved

bluestreak01 and others added 4 commits January 13, 2026 23:19

more tests

b1ff100

Merge branch 'master' into vi_ksum

c172677

Merge remote-tracking branch 'origin/master' into vi_ksum

3ef29df

address reviews, consolidated window function tests in one class

13459ca

Merge branch 'master' into vi_ksum

cf1e231

bluestreak01 merged commit 79d10ed into master Jan 14, 2026
43 checks passed

bluestreak01 deleted the vi_ksum branch January 14, 2026 16:19

coderabbitai bot mentioned this pull request Jan 15, 2026

feat(sql): implement EMA window function via avg() #6650

Merged

10 tasks

bluestreak01 mentioned this pull request Jan 15, 2026

docs(window-functions): add EMA, VWEMA, and ksum() window functions questdb/documentation#328

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sql): implement ksum() window function with Kahan summation#6642

feat(sql): implement ksum() window function with Kahan summation#6642
bluestreak01 merged 7 commits intomasterfrom
vi_ksum

bluestreak01 commented Jan 13, 2026

Uh oh!

coderabbitai bot commented Jan 13, 2026 •

edited

Loading

Review skipped

Uh oh!

bluestreak01 commented Jan 13, 2026

Uh oh!

coderabbitai bot commented Jan 13, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

bluestreak01 commented Jan 14, 2026

Uh oh!

coderabbitai bot commented Jan 14, 2026

Uh oh!

glasstiger commented Jan 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bluestreak01 commented Jan 13, 2026

Summary

Changes

Usage Examples

Test plan

Uh oh!

coderabbitai bot commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

bluestreak01 commented Jan 13, 2026

Uh oh!

coderabbitai bot commented Jan 13, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bluestreak01 commented Jan 14, 2026

Uh oh!

coderabbitai bot commented Jan 14, 2026

Uh oh!

glasstiger commented Jan 14, 2026

[PR Coverage check]

file detail

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Jan 13, 2026 •

edited

Loading