Optimize segment SQL when segment subqueries are used

[diosmosis]

Description:

Fixes #20467

Changes:

• Refactor segment parsing and related code to use an expression tree (as nested array) for the intermediate data structures. First level in the array represents AND-ed expressions, the second level represents OR-ed expressions. This makes it easier to manipulate the expression before converting it to SQL and simplifies the related code.
• Merge adjacent NOT IN segment subqueries so fewer exist in segment SQL overall. Segment subqueries that are next to each other in a single OR expression chain are merged, and segment subqueries that are not parts of OR expression chains but are part of the overall AND expression chain are merged.

Review

• Functional review done
• Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• Security review done
• Wording review done
• Code review done
• Tests were added if useful/possible
• Reviewed for breaking changes
• Developer changelog updated if needed
• Documentation added if needed
• Existing documentation updated if needed

[michalkleiner]

I've looked through the change and I can't confidently give it an approval on its merits, I could only do it on the fact the tests are passing, but the tests were adjusted as well, so I would need more time or defer to someone else.

[bx80]

@sgiehl Now the urlencoding issue is resolved, could you take another look at reviewing this PR when you get a chance? Thanks! 🙂

[sgiehl]

In general the code looks fine to me. But as mentioned before it's really hard to ensure if it might introduce any possible regressions regarding the resulting data.
To minimize the risk I did some more local testing by creating a bunch of fake visits for a certain day. Then I created various different segments and added some logging to receive the amount of rows inserted in the temporary segment table. Then I ran archiving once without the config flag and once with it and compared the numbers. For my local tests the numbers were the same. We nevertheless could so some similar testing with bigger data sets or maybe really compare the resulting visit ids to ensure there are no differences.

[github-actions]

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

[bx80]

I've reverted the feature flag change and merged in Michal's improved parsing code. If somebody wants to give this a last quick check over then it should be ready to merge into 5.x-dev 🏁

[michalkleiner]

Ben and Stefan tested this extensively and we have done some more tests in the cloud environment without any obvious issues, so I guess that's as much as we can do at this stage.