Skip to content

Fix a few bugs in vParquet2 structural operators#2660

Merged
mdisibio merged 4 commits intografana:mainfrom
mdisibio:fix-structural-uninitialized-data
Jul 14, 2023
Merged

Fix a few bugs in vParquet2 structural operators#2660
mdisibio merged 4 commits intografana:mainfrom
mdisibio:fix-structural-uninitialized-data

Conversation

@mdisibio
Copy link
Copy Markdown
Contributor

@mdisibio mdisibio commented Jul 14, 2023

What this PR does:
A few fixes in the structural operators:

  1. Need to skip over spans with broken nesting. This is detected as any value of either span == 0. Without this it was falsely identifying matches on partial traces where the spans had broken nesting
  2. Sibling operator would falsely identify a span as its own sibling if it was present in multiple spansets. I.e. { .foo} ~ {.foo} because each input technically had the same parent. There are two ways to fix this, and I chose the more efficient one. Theoretically we could loading both Parent and Left and ensure Lefts are different (not same spans). However instead I am just checking struct pointers to ensure not the same *span. This works until the TraceQL engine starts making multiple passes over a file (which is not the plan and may be never).
  3. There was also a small typo in DescendantOf that could have led to more false positives. I never saw this in the wild.

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

Checklist

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

// Spans with missing data, never a match.
return false
}
return s.nestedSetLeft > ss.nestedSetLeft && s.nestedSetRight < ss.nestedSetRight
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The typo was the third var in this statement s.nestedSetLeft -> s.nestedSetRight

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.

Child operator sometimes returns incorrect results

2 participants