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

WIP: Proposal to address mixed-type attribute querying limitations #4391

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ndk
Copy link
Contributor

@ndk ndk commented Nov 27, 2024

What this PR does:
Below is my understanding of the current limitations. Please feel free to correct me if I’ve misunderstood or overlooked something.

Attributes of the same type are stored in the same column. For example, integers are stored in one column and floats in another.

Querying operates in two stages:

  • Predicate Creation: Predicates are created based on the operand types.
  • Chunk Scanning: Chunks are scanned, and spans are filtered using the predicates.

The issue arises because predicates are generated based on the operand type. If an attribute is stored as a float but the operand is an integer, the predicate evaluates against the integers column instead of the floats column. This results in incorrect behavior.

Proposed Solution
The idea is to generate predicates for both integers and floats, allowing both columns to be scanned for the queried attribute.

In this PR, I’ve created a proof-of-concept by copying the existing createAttributeIterator function to createAttributeIterator2. This duplication is intentional, as the original function is used in multiple places, and I want to avoid introducing unintended side effects until the approach is validated.

case traceql.TypeInt:
	{
		pred, err := createIntPredicate(cond.Op, cond.Operands)
		if err != nil {
			return nil, fmt.Errorf("creating attribute predicate: %w", err)
		}
		attrIntPreds = append(attrIntPreds, pred)
	}

	{
		if i, ok := cond.Operands[0].Int(); ok {
			operands := traceql.Operands{traceql.NewStaticFloat(float64(i))}
			pred, err := createFloatPredicate(cond.Op, operands)
			if err != nil {
				return nil, fmt.Errorf("creating attribute predicate: %w", err)
			}
			attrFltPreds = append(attrFltPreds, pred)
		}
	}

case traceql.TypeFloat:
	{
		operands := traceql.Operands{traceql.NewStaticInt(int(cond.Operands[0].Float()))}
		pred, err := createIntPredicate(cond.Op, operands)
		if err != nil {
			return nil, fmt.Errorf("creating attribute predicate: %w", err)
		}
		attrIntPreds = append(attrIntPreds, pred)
	}

	{
		pred, err := createFloatPredicate(cond.Op, cond.Operands)
		if err != nil {
			return nil, fmt.Errorf("creating attribute predicate: %w", err)
		}
		attrFltPreds = append(attrFltPreds, pred)
	}

WDYT? :)

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

Checklist

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

@ndk ndk changed the title WIP: Proposal to address mixed-type attribute querying limitations TraceQL: Proposal to address mixed-type attribute querying limitations Nov 27, 2024
@ndk ndk changed the title TraceQL: Proposal to address mixed-type attribute querying limitations WIP: Proposal to address mixed-type attribute querying limitations Dec 19, 2024
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.

TraceQL: Ints can't be compared to floats
1 participant