Repository navigation
Fix NULL handling in VAR, STDEV, VARP, STDEVP, STD and STDDEV (part of #2481) - #2581
Open
joshuablac wants to merge 1 commit into
Open
joshuablac wants to merge 1 commit into
joshuablac wants to merge 1 commit into
Conversation
…SQL#2481 VARP (and STDEVP, STD, STDDEV through it) counted NULL as 0, so VARP over [1, 2, 3, 4, NULL] gave 2 instead of 1.25. VAR and STDEV only skipped null, so a row with a missing value (undefined) turned every variance aggregate into NULL. An all-NULL group gave 0. All six aggregates now skip NULL values (null, undefined or NaN) and return NULL (undefined) when there is nothing to aggregate. The sample variance (VAR, STDEV) is NULL for fewer than two values, as in SQL. The duplicate definition of STDEV is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #2481. This fixes its
VARandVARPsections and the duplicateSTDEVdefinition. TheGREATEST/LEAST, windowMIN/MAXandQUARTsections are not touched; #2479 and #2480 already cover MIN/MAX.Problem
The variance aggregates don't skip NULL values:
[1, 2, 3, 4]plus…developNULLVARP/STDEVP/STD/STDDEV2/1.414(NULL counted as 0)1.25/1.118undefined)undefined(NaN)0undefined(NULL)5)VAR/STDEV0undefined(NULL)VARandSTDEValready skippednull, but notundefined, which is how AlaSQL usually represents a missing value.VARPskipped neither.Fix
In
src/55functions.js:VARandVARPskip NULL values (null,undefined, orNaN, the same values the operator null check treats as NULL).STDEV,STDEVP,STDandSTDDEVdelegate to them.undefined(NULL) instead of0, likeAVG, and likeVAR_POP/VAR_SAMPin SQL.VAR,STDEV) is NULL for fewer than two values, as SQL'sVAR_SAMP/STDDEV_SAMPare. The population variance of a single value stays0.STDEVwas defined twice with identical bodies; the duplicate is removed.Behavior changes to note:
VAR/STDEVof a single value goes from0toundefined.0toundefined.No existing test depended on the old values.
Tests
test/test2481.js:developand all pass with this change. The "without NULL" case passes either way and guards the formulas.bun test, same command asyarn test-only): 2255 pass, 213 skip. The only failures are the 2 CLI tests intest2149.js, which fail the same way on unmodifieddevelopin my local setup (a space in my checkout path, and the version-injection step ofbuild.shnot running).This fix was written with AI assistance (Claude Code) and verified locally as described above.
🤖 Generated with Claude Code