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

Revert pipeline processor invalid field access check due to regression #18322

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

patrickmann
Copy link
Contributor

@patrickmann patrickmann commented Feb 20, 2024

Resolves #18017

#16029 was designed to flag illegal expressions like <array>.[<index>] in a pipeline rule.
However, we end up also blocking legal expressions, e.g. to access JSON fields of type array. JSON parsing allows using dot notation or square brackets interchangeably.

This PR reverts the change.

How Has This Been Tested?

Parser behaves as before:

  • Able to access GeoIP subdivisions data in a pipeline rule
  • rs.["a"] does not trigger a syntax error; error message visible in server log

@patrickmann patrickmann marked this pull request as ready for review February 20, 2024 10:47
@patrickmann patrickmann requested a review from a team February 20, 2024 10:48
@bernd bernd requested a review from kroepke February 20, 2024 10:50
@moesterheld
Copy link
Contributor

lgtm. unfortunately, i can't think of a way to distinguish the two use cases either

@patrickmann patrickmann merged commit 3751137 into master Mar 7, 2024
6 checks passed
@patrickmann patrickmann deleted the revert/16196 branch March 7, 2024 07:07
patrickmann added a commit that referenced this pull request Mar 7, 2024
#18322)

* revert PR 16196

* CL

---------

Co-authored-by: Matthias Oesterheld <[email protected]>

(cherry picked from commit 3751137)
moesterheld pushed a commit that referenced this pull request Mar 15, 2024
#18322) (#18523)

* revert PR 16196

* CL

---------

Co-authored-by: Matthias Oesterheld <[email protected]>

(cherry picked from commit 3751137)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Geo subdivisions in pipeline no longer work
2 participants