Skip to content

Conversation

@kbrabrand
Copy link
Contributor

@kbrabrand kbrabrand commented Dec 9, 2025

TL;DR

Add addition of dateTime to number to the spec to reflect the actual implementation.

What changed?

The spec currently defines dateTime + number ⇒ dateTime and number + dateTime ⇒ null, while it has been implemented as a commutative operation in the query engine, meaning that the order of the operands is irrelevant.

With this change the spec defines this as a commutative operation, reflecting the actual implementation.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kbrabrand kbrabrand marked this pull request as ready for review December 9, 2025 08:42
@kbrabrand kbrabrand requested a review from atombender December 9, 2025 08:51
Copy link
Member

@atombender atombender left a comment

Choose a reason for hiding this comment

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

LGTM. Have you confirmed this is the currently implemented behaviour in the query API and GROQ.js?

Copy link
Contributor Author

kbrabrand commented Dec 9, 2025

Yes. The change was prompted by a user reporting that groq-js didn't behave as expected with arithmetic operations on dateTimes, and while adjusting I found that the implementation didn't match the spec.

image.png

@atombender
Copy link
Member

Great!

Copy link
Contributor Author

kbrabrand commented Dec 9, 2025

Merge activity

  • Dec 9, 7:33 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Dec 9, 7:33 PM UTC: Graphite couldn't merge this PR because it failed for an unknown reason (Merge commits are not allowed on this repository).

@kbrabrand kbrabrand merged commit 9b0466a into main Dec 9, 2025
5 checks passed
@kbrabrand kbrabrand deleted the docs/cldx-4469/commutative-addition-number-datetime branch December 9, 2025 19:34
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.

2 participants