Skip to content

Add test case for aggregation $abs operator - #100

Open
noisersup wants to merge 3 commits into
OpenDocDB:mainfrom
noisersup:test-aggregation-operator-abs
Open

noisersup wants to merge 3 commits into
OpenDocDB:mainfrom
noisersup:test-aggregation-operator-abs

Conversation

@noisersup

Copy link
Copy Markdown
Collaborator

Closes #57.

@noisersup noisersup self-assigned this Jun 9, 2025
@noisersup
noisersup requested a review from chilagrow June 9, 2025 15:38
@noisersup
noisersup marked this pull request as ready for review June 9, 2025 15:38
Copilot AI review requested due to automatic review settings June 9, 2025 15:38
@noisersup
noisersup requested a review from AlekSi as a code owner June 9, 2025 15:38
@noisersup
noisersup enabled auto-merge (squash) June 9, 2025 15:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a comprehensive test case file for the aggregation "$abs" operator, ensuring that the operator behaves correctly across various numeric types.

  • Adds a new test file (abs.json) with cases for int, long, double, and decimal values
  • Validates edge cases, including maximum, minimum, zero, and infinity values

"decimal"
],
"$gt": {
"$numberLong": "-9223372036854775808"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

$match is already using $type filter, why do you need this $gt condition? Can we remove it? $abs is able to handle numbers, null and missing field.

@noisersup noisersup Jun 10, 2025 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The absolute value of int64-min (-9223372036854775808), overflows int64, which results in error.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I suggest matching specific _id that case. Because float64 could have a smaller valid number right?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@noisersup this comment wasn't addressed 👋

Comment thread cts/aggregation/operators/abs.json Outdated
@noisersup
noisersup requested a review from chilagrow June 10, 2025 12:54

@chilagrow chilagrow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See comment 🙏

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.

Add test cases for aggregations

3 participants