Type conditioned fetching

[o0Ignition0o]

Type conditioned fetching (PR #4748)

Type conditioned fetching

When querying a field that is in a path of 2 or more unions, the query planner was not able to handle different selections and would aggressively collapse selections in fetches yielding an incorrect plan.

This change introduces an experimental configuration option to enable type conditioned fetching:

experimental_type_conditioned_fetching: true # false by default

By @o0Ignition0o in #4748

[router-perf]

CI performance tests

• reload - Reload test over a long period of time at a constant rate of users
• events_big_cap_high_rate_callback - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity using callback mode
• events_without_dedup_callback - Stress test for events with a lot of users and deduplication DISABLED using callback mode
• large-request - Stress test with a 1 MB request payload
• const - Basic stress test that runs with a constant number of users
• no-graphos - Basic stress test, no GraphOS.
• step-jemalloc-tuning - Clone of the basic stress test for jemalloc tuning
• events - Stress test for events with a lot of users and deduplication ENABLED
• events_callback - Stress test for events with a lot of users and deduplication ENABLED in callback mode
• events_big_cap_high_rate - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity
• events_without_dedup - Stress test for events with a lot of users and deduplication DISABLED
• xxlarge-request - Stress test with 100 MB request payload
• xlarge-request - Stress test with 10 MB request payload
• step - Basic stress test that steps up the number of users over time

[garypen]

NIT: Maybe easier to understand when captures are used if these are named.

[o0Ignition0o]

fixed in d14d077

[garypen]

I'm not sure this is really fixed. I can see that you've named one of the groups "", but I can't see where it's being used. It looks like you are still using the indices of the various captures.

e.g.:

        let path = TYPE_CONDITIONS_REGEX.replace(s, |caps: &Captures| {
            type_conditions.extend(
                caps.extract::<1>()
                    .1
                    .map(|s| s.split(',').map(|s| s.to_string()))
                    .into_iter()
                    .flatten(),
            );
            ""
        });

You are always using extract, group 1, which is a bit opaque. If you want to use the name then it would be something like: caps.name("condition") and you'd need to do some handling for not being present.

Anyway, it was a NIT, so I'm fine with the extract approach that you are using, in which case no need to name the capture group.

If you do think it's worth naming the various capture groups, then you should use them to make the code a bit less opaque when you are pulling out the conditions as I outlined above.

[o0Ignition0o]

actually fixed in 6c67cac 😁

[garypen]

Now you are making me feel really picky, but there multiple used of extract(). May as well change all of them.

[o0Ignition0o]

Let's followup on that!