wrong response when using `()` and operators in filters

[avinal]

Expected Behavior

The command or API call using || in filter should respect the OR operator. For example, on running this command

$ ./tkn-results records list default/results/- --insecure --filter="data_type==TASK_RUN && (data.spec.pipelineSpec.tasks[0].name=='hello'||data.metadata.name=='hell')" 

the response should only show TaskRuns that fulfill one or both other conditions.

Actual Behavior

$ ./tkn-results records list default/results/- --insecure --filter="data_type==TASK_RUN && (data.spec.pipelineSpec.tasks[0].name=='hello'||data.metadata.name=='hell')" 
Name                                                                                               Type                                    Start                                   Update
default/results/c030918c-b7ff-4fd9-bafb-56487c481aed/records/c030918c-b7ff-4fd9-bafb-56487c481aed  tekton.dev/v1beta1.PipelineRun          2023-09-22 19:27:06 +0530 IST           2023-09-22 19:27:31 +0530 IST

It seems the CEL converter is not respecting the () boundary.

Steps to Reproduce the Problem

1. Needs Tekton Results installation with some data in database
2. Run a command or send an API request involving brackets and operators.
3. Observe the response

Additional

Reported by @karthikjeeyar

[avinal]

/assign

[avinal]

Here is what I discovered while debugging.

(assume expr = data.metadata.namespace == "foo" && (data.metadata.name == "bar" || data.metadata.name == "baz"))

• The AST is formed correctly for the filter expr
• The resulting SQL expression for the given expr should be (data->'metadata'->>'namespace') = 'foo' AND ((data->'metadata'->>'name') = 'bar' OR (data->'metadata'->>'name') = 'baz')
• But instead it is (data->'metadata'->>'namespace') = 'foo' AND (data->'metadata'->>'name') = 'bar' OR (data->'metadata'->>'name') = 'baz'
• The AST will look like this
• which is the expr data.metadata.namespace == "foo" && data.metadata.name == "bar" || data.metadata.name == "baz"

Where it is getting wrong

• The first arg (left side of &&) and the second arg (subexpr with ||) are recognized correctly
• But while interpreting second arg, the ( provided in the actual expr is not being added hence the final SQL output does not has any parentheses and the whole expression is considered one giant expr.

Possible fixes

1. Just add parentheses whenever a binary expr is interpreted. This solves the problem but adds extra redundant parentheses to the SQL output. No negative impact on anything.
2. implement a check to detect subexpression and add parentheses whenever detected. Complex to implement but removes extra parentheses.