Skip to content

fix count aggregate function (evaluate only not null fields)#453

Merged
samwillis merged 4 commits intoTanStack:mainfrom
fcannizzaro:fix-agg-count-only-not-null
Sep 8, 2025
Merged

fix count aggregate function (evaluate only not null fields)#453
samwillis merged 4 commits intoTanStack:mainfrom
fcannizzaro:fix-agg-count-only-not-null

Conversation

@fcannizzaro
Copy link
Copy Markdown
Contributor

@fcannizzaro fcannizzaro commented Aug 26, 2025

The current count aggregate function also counts NULL values (unlike the SQL count).

Ex.

[
	{ category: "A", amount: 10 },
	{ category: "B", amount: 10 },
	{ category: "A", amount: null },
	{ category: "B", amount: null },
]

This code below

const { data } = useLiveQuery((q) =>
  q
    .from({ t: collection })
    .groupBy(({ t }) => t.category)
    .select(({ t }) => ({
      category: t.category,
      total: count(t.category),
      notNull: count(t.amount)
    }))
);

returned

[
	{ category: "A", total: 2, notNull: 2 },
	{ category: "B", total: 2, notNull: 2 },
]

now it gives

[
	{ category: "A", total: 2, notNull: 1 },
	{ category: "B", total: 2, notNull: 1 },
]

I have also added a new test case that keep track of this behavior to prevent a future regression.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Aug 26, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: cb976c6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@tanstack/db-ivm Patch
@tanstack/db Patch
@tanstack/electric-db-collection Patch
@tanstack/query-db-collection Patch
@tanstack/react-db Patch
@tanstack/solid-db Patch
@tanstack/svelte-db Patch
@tanstack/trailbase-db-collection Patch
@tanstack/vue-db Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Aug 26, 2025 •
edited
Loading

More templates

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@453

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@453

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@453

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@453

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@453

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@453

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@453

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@453

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@453

commit: cb976c6

samwillis
samwillis requested changes Aug 26, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@samwillis samwillis left a comment

Choose a reason for hiding this comment

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

This is great, thanks!

  • It needs tests both with and without a value extractor in the main query aggregate tests in the main db package (to go along with the db-ivm test)
  • we need a changeset with a brief summary off the change.

Comment thread packages/db-ivm/src/operators/groupBy.ts
@fcannizzaro fcannizzaro requested a review from samwillis August 27, 2025 12:18
samwillis
samwillis approved these changes Sep 8, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@samwillis samwillis left a comment

Choose a reason for hiding this comment

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

Sorry it's taken a little time to get to this, it's perfect. I'm merging now

@samwillis samwillis merged commit 08303e6 into TanStack:main Sep 8, 2025
6 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 8, 2025
Merged
@fcannizzaro fcannizzaro deleted the fix-agg-count-only-not-null branch September 8, 2025 16:04
Uziniii pushed a commit to Uziniii/db that referenced this pull request Sep 19, 2025
@fcannizzaro @samwillis @Uziniii
fix count aggregate function (evaluate only not null fields) (TanStac…
740858f 
…k#453)

Co-authored-by: Sam Willis <[email protected]>
Uziniii pushed a commit to Uziniii/db that referenced this pull request Sep 19, 2025
@fcannizzaro @samwillis @Uziniii
fix count aggregate function (evaluate only not null fields) (TanStac…
3163d8e 
…k#453)

Co-authored-by: Sam Willis <[email protected]>
Uziniii pushed a commit to Uziniii/db that referenced this pull request Sep 19, 2025
@fcannizzaro @samwillis @Uziniii
fix count aggregate function (evaluate only not null fields) (TanStac…
f046cb6 
…k#453)

Co-authored-by: Sam Willis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samwillis samwillis samwillis 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.

2 participants

@fcannizzaro @samwillis