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(AIP-160): clarify unset field in traversal behavior #1469

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

noahdietz
Copy link
Collaborator

Clarify the expected behavior when a non-primitive field in a traversal operator's chain of fields is not set, it should be skipped i.e. not matched. This has been the expected behavior since inception, but was undocumented.

Internal bug http://b/388065878

@noahdietz noahdietz requested a review from a team as a code owner January 7, 2025 19:48
@noahdietz noahdietz requested a review from neomagus00 January 7, 2025 19:48
@@ -138,6 +138,12 @@ the service **should** return an error with `INVALID_ARGUMENT`. A service
**may** permit traversal to undefined keys on maps and structs, and **should**
document how it behaves in this situation.

When evaluating an expression involving a traversal, if any non-primitive field
Copy link
Contributor

Choose a reason for hiding this comment

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

What about if the comparison is a "not equal" one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Regardless of the comparison in that case, it is skipped, at least that is my understanding of the feedback I got from maintainers. Unset == no value comparison at all.

I believe this is because of the ambiguity of the intent in such a filter - do they want to filter for entries that have a set and a.b != "foo" OR do they want to filter for entries that either do not have a set or a set and a.b != "foo".

This is where a Has operator would come into play e.g. "has: a AND a.b != "foo""

@aip-dev aip-dev deleted a comment from AKSHAY1469-gli Jan 13, 2025
@aip-dev aip-dev deleted a comment from AKSHAY1469-gli Jan 13, 2025
@noahdietz noahdietz requested review from jskeet and removed request for neomagus00 January 13, 2025 19:49
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.

2 participants