Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TraceQL: make nil type symmetric #3991

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

electron0zero
Copy link
Member

What this PR does:
makes the nil symmetric.

Which issue(s) this PR fixes:
Fixes #3989

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@electron0zero electron0zero changed the title fix: make nil type symmetric TraceQL: make nil type symmetric Aug 21, 2024
@electron0zero electron0zero marked this pull request as ready for review August 21, 2024 23:29
@electron0zero electron0zero self-assigned this Aug 21, 2024
@electron0zero
Copy link
Member Author

After some digging into the failing test, I have arrived at a question that boils down to, “should a query like nil != 0 return data or not?”

I’m unclear about the expected behavior for queries nil != 0, or nil != 5, should they evaluate to true or false?. This behavior isn’t currently defined, and we need to establish a clear definition.

Even if we prevent queries TypeNil != TypeInt or TypeNil != TypeFloatat the parse layers, users can write queries that end up evaluating uptoTypeNil != TypeInt`.

For instance, avg(.foo) (foo doesn't exist) is not evaluated to nil. so a query like { } | avg(.dne) != 0 will finally be evaluated to nil != 0

We introduced the nil type to allow easy checks for non-existent attributes using queries like { .foo != nil }. If we want to maintain symmetry and allow queries like { nil != .foo }, we need to clearly define the behavior of nil.

Also a follow up question: should functions be evaluating to nil, should we be evaluating avg(.foo) to nil, if foo doesn't exist?

Copy link
Contributor

This PR has been automatically marked as stale because it has not had any activity in the past 60 days.
The next time this stale check runs, the stale label will be removed if there is new activity. This pull request will be closed in 15 days if there is no new activity.
Please apply keepalive label to exempt this Pull Request.

@github-actions github-actions bot added the stale Used for stale issues / PRs label Oct 22, 2024
@joe-elliott joe-elliott removed the stale Used for stale issues / PRs label Oct 22, 2024
@electron0zero electron0zero added the keepalive Label to exempt Issues / PRs from stale workflow label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keepalive Label to exempt Issues / PRs from stale workflow
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

[TraceQL] != nil is not symmetric
2 participants