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

fix: handle exists and nexists for mat columns and top level columns #6672

Closed
wants to merge 1 commit into from

Conversation

nityanandagohain
Copy link
Member

@nityanandagohain nityanandagohain commented Dec 19, 2024

Fixes #6584

Now the logic for exists/nexists is similar to that of logs. For top level columns it will try to check if it's empty string or number is not equal to zero.

For materialized columns it will use the exists column for that materialized column.

This needs to be released before releasing this


Important

Refactor exists/not exists handling for top-level and materialized columns in query_builder.go, updating tests accordingly.

  • Behavior:
    • Updated logic for exists and not exists operators in query_builder.go to handle both top-level and materialized columns.
    • For top-level columns, checks if string is empty or number is not zero.
    • For materialized columns, uses the exists column.
  • Functions:
    • Added getExistsNexistsFilter() in query_builder.go to centralize exists/not exists logic.
    • Removed existsSubQueryForFixedColumn() as its logic is now in getExistsNexistsFilter().
  • Tests:
    • Updated Test_buildTracesFilterQuery and Test_buildTracesQuery in query_builder_test.go to reflect new exists/not exists logic.
    • Adjusted expected outputs in tests to match new query format.

This description was created by Ellipsis for 161f5fd. It will automatically update as commits are pushed.

@nityanandagohain nityanandagohain marked this pull request as draft December 19, 2024 06:21
@github-actions github-actions bot added the bug Something isn't working label Dec 19, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 161f5fd in 2 minutes and 1 seconds

More details
  • Looked at 123 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. pkg/query-service/app/traces/v4/query_builder.go:28
  • Draft comment:
    Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_JbcoJwqxCCdoWqLz


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@pranay01 pranay01 deleted the branch develop December 19, 2024 13:03
@pranay01 pranay01 closed this Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add endpoint for attributes materialization for traces
2 participants