Monotonic select #18546

frankmcsherry · 2023-04-01T15:49:44Z

Demonstration of one approach to introducing monotonicity for SELECT queries, by informing LIR plans of consolidation that when performed should result in collections with non-negative updates.

Motivation

Tips for reviewer

Checklist

This PR has adequate test coverage / QA involvement has been duly considered.
This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
This PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way) and therefore is tagged with a T-proto label.
If this PR will require changes to cloud orchestration, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
This PR includes the following user-facing behavior changes:

CLAassistant · 2023-04-01T15:49:49Z

All committers have signed the CLA.

vmarcos · 2023-04-03T16:58:59Z

An evaluation of the first two commits in this PR can be found in this spreadsheet. In contrast to the evaluation of PR #14607, we included here a few additional queries with TopK after Reduce, which this PR can convert to monotonic evaluation while the former one could not. The additional queries were adapted from the definitions in epic #16698. They included some scalar expressions to compute the input to Reduce in them, which we are not so good at, so a second pass at the numbers could be taken where we compute single-attribute aggregates to focus on the speedups obtainable for the core operators in focus here.

vmarcos · 2023-04-04T16:42:07Z

Important note for follow-up work: The plans produced here still sometimes set must_consolidate to true when not strictly necessary. Consider the following plan, obtained by forcing single-timestamp planning in sequence_explain_plan, for a one-shot SELECT:

materialize=> explain physical plan for
select
    sum(revenue), count(l_orderkey)
from (
    select l_orderkey,
           sum(l_extendedprice *
             (1 - l_discount)) as revenue
    from lineitem
    group by l_orderkey
    order by revenue desc 
    limit 10);
                                                    Physical Plan                                                    
---------------------------------------------------------------------------------------------------------------------
 Explained Query:                                                                                                   +
   Return                                                                                                           +
     Union                                                                                                          +
       ArrangeBy                                                                                                    +
         input_key=[]                                                                                               +
         raw=true                                                                                                   +
         Get::PassArrangements l0                                                                                   +
           raw=false                                                                                                +
           arrangements[0]={ key=[], permutation=id, thinning=(#0, #1) }                                            +
       Mfp                                                                                                          +
         project=(#0, #1)                                                                                           +
         map=(null, 0)                                                                                              +
         Union                                                                                                      +
           Negate                                                                                                   +
             Get::Arrangement l0                                                                                    +
               project=()                                                                                           +
               key=                                                                                                 +
               raw=false                                                                                            +
               arrangements[0]={ key=[], permutation=id, thinning=(#0, #1) }                                        +
           Constant                                                                                                 +
             - ()                                                                                                   +
   With                                                                                                             +
     cte l0 =                                                                                                       +
       Reduce::Accumulable                                                                                          +
         simple_aggrs[0]=(0, 0, sum(#0))                                                                            +
         simple_aggrs[1]=(1, 1, count(*))                                                                           +
         val_plan                                                                                                   +
           project=(#0, #1)                                                                                         +
           map=(true)                                                                                               +
         key_plan                                                                                                   +
           project=()                                                                                               +
         TopK::MonotonicTopK order_by=[#0 desc nulls_first] limit=10        must_consolidate                        +
                                                                                                                    +
           Mfp                                                                                                      +
             project=(#1)                                                                                           +
             input_key=#0                                                                                           +
             Reduce::Accumulable                                                                                    +
               simple_aggrs[0]=(0, 0, sum((#1 * (1 - #2))))                                                         +
               val_plan                                                                                             +
                 project=(#3)                                                                                       +
                 map=((#1 * (1 - #2)))                                                                              +
               key_plan                                                                                             +
                 project=(#0)                                                                                       +
               Get::Arrangement tpch.public.lineitem                                                                +
                 project=(#0, #5, #6)                                                                               +
                 key=#0, #3                                                                                         +
                 raw=false                                                                                          +
                 arrangements[0]={ key=[#0, #3], permutation={#1: #2, #2: #3, #3: #1}, thinning=(#1, #2, #4..=#15) }+
                                                                                                                    +
 Used Indexes:                                                                                                      +
   - tpch.public.pk_lineitem_orderkey_linenumber                                                                    +
 
(1 row)

Above, we see that must_consolidate is set for the TopK::MonotonicTopK operator even though its input is coming from a Reduce::Accumulable. Since the latter already requires arranged data, we would not need to reconsolidate the data in the former.

vmarcos · 2023-04-05T15:54:46Z

I tried to schedule SQL-related nightly runs for this PR; however, all of the errors found are also happening in main at the moment, and none appear to be related to the PR.

antiguru

Suggesting changes for an explain oversight!

src/compute-client/src/explain/text.rs

vmarcos · 2023-04-06T17:13:05Z

@aalexandrov @teskje @jkosh44 I am tagging you as reviewers for this PR. The PR is an MVP for the feature of exploiting monotonic operator variants in single-time queries (i.e., one-shot SELECTs currently). There are a few items that I'd like to explicitly leave to follow-up work, if you agree, namely:

Setting must_consolidate to false in one-shot SELECTs where consolidation in the monotonic operator is unnecessary (for an example, see Monotonic select #18546 (comment)).
Extended validation and improvements of EXPLAIN output. Now, EXPLAIN PHYSICAL PLAN will not show the plan produced for a one-shot SELECT with the monotonic operators. Even when that is fixed, MIR-level EXPLAIN will for TopK say that monotonic=false, but the physical plan will differ in the actual operator choice. This follow-up work should be enabled by EXPLAIN pipeline should reflect different optimization paths #18089.
Fixing of our tests to ensure that we cover both non-monotonic and monotonic operator choices in CI. Right now, the PR should be inert in that respect since we are gating the changes behind a feature flag, turned off by default. But if we ever decide to flip this feature flag on and eventually retire it, we need a permanent solution for ensuring that our tests exercise the different operator variants.

I can create follow-up issues for (1) and (3) above if you are in agreement. Even in its current version, we can already measure quite sizable query processing times gains for some queries (when the feature flag is turned on).

@frankmcsherry Hope that it is OK with you to initiate review for this PR; otherwise, please let us know what other strategy to take!

src/sql/src/session/vars.rs

jkosh44

Surfaces changes LGTM

vmarcos · 2023-04-10T12:02:46Z

Just to be on the safe side, I scheduled a slow SQL Logic Test run on commit 8895d80, prior to introducing the feature flag. No errors were reported, so it looks like doing operator selection for one-shot SELECTs as in this PR passes this first level of validation: https://buildkite.com/materialize/sql-logic-tests/builds/5507.

frankmcsherry · 2023-04-10T18:18:42Z

I've been told this may be blocked on me! It shouldn't; if it's good to go, go for it!

teskje

LGTM! I was wondering about the risk involved with removing those "always consolidates". But with the ensure_consolidates we won't produce incorrect data, so that seems fine.

teskje · 2023-04-11T09:03:56Z

src/adapter/src/coord/dataflows.rs

                }
            }
+            let plan = self.finalize_dataflow(dataflow, instance)?;


It feels very brittle that we have to rely on nobody modifying the finalized plan anymore for correctness. The "lock down the COMPUTE interface" part of #18496 can solve this, but I wonder if there is a way to short-term ensure that plan is not modified accidentally.

I agree this is something to give some thought to as part of #18496, but I am afraid that trying to address this locally might exceed the scope of this PR. Still, we are introducing an unwritten API requirement here that until needs to be assigned prior to calling Coordinator::finalize_dataflow, which was not previously present. I wrote down this extra requirement now in the docstring of Coordinator::finalize_dataflow.

src/compute-client/src/plan/mod.rs

aalexandrov

Looks good overall, I have some minor questions / nits about naming and code organization.

src/compute-client/src/plan/top_k.proto

src/compute-client/src/plan/top_k.rs

src/compute-client/src/plan/mod.rs

aalexandrov · 2023-04-12T16:47:54Z

src/compute/src/render/top_k.rs

+                    }
+
+                    let debug_name = self.debug_name.to_string();
+                    let (collection, errs) = collection.ensure_monotonic(move |data, diff| {


I thought that this check was added in #18415.

The check was added by #18140 to both monotonic top-1 and to monotonic reduce; however, it was not added there to monotonic top-k. This is because monotonic top-k does not need to use explode_one, and adding explode_one to the other two cases was the intended change of #18140.

So the present PR complements that work and makes sure that we have an ensure_monotonic check in all of our monotonic rendering paths.

src/compute/src/render/top_k.rs

aalexandrov · 2023-04-12T16:52:57Z

src/adapter/src/coord/sequencer/inner.rs

-                    .map_err(AdapterError::Internal)
+                Plan::<mz_repr::Timestamp>::finalize_dataflow(
+                    dataflow,
+                    self.catalog()


I guess without tweaking the dataflow construction logic this is not sufficient for triggering the refine_single_time_dataflow call.

Yes, that is correct. I did not change the setting of until here for dataflow, because otherwise, we'd show one-shot SELECT plans on EXPLAIN PHYSICAL PLAN when the feature flag is enabled. I propose that we leave figuring out the right output to produce for EXPLAIN as part of #18089.

This commit fixes the invalid accumulation tests in the mzcompose cluster test workflows to use non-monotonic rendering whenever a min/max aggregate is employed. This is ensured by use of materialized views instead of one-off SELECTs in the tests, since the latter now exploit logical monotonicity.

This commit fixes the negative multiplicities tests in testdrive to use non-monotonic rendering whenever a min/max aggregate is employed. This is ensured by use of indexed views instead of one-off SELECTs in the tests, since the latter now exploit logical monotonicity.

This commit adds a monotonicity check with the EnsureMonotonic operator to the rendering of monotonic top-k. This step increases the consistency of the rendering code across monotonic top-k, monotonic top-1, and monotonic hierarchical reductions.

This commit introduces the feature flag enable_monotonic_oneshot_selects, turned off by default, and gates single-time operator selection to use monotonic plans for one-shot SELECTs behind this flag. As a consequence, after this commit, all tests in CI will by default execute against non-monotonic plans, as previously. The commit enables us to carefully evaluate when to make the functionality of monotonic plans for one-shot SELECTs available by default.

This commit includes code improvements to refactor plan refinements performed at the end of Plan::finalize_dataflow into separate functions, improving readability. In addition, the commit: (a) makes explicit in the documentation of the API of Coordinator::finalize_dataflow the new requirement to set the until field of the dataflow, and (b) adds a method to DataflowDescription to test if a dataflow refers to a single timestamp.

This commit refactors the method flag_snapshot added in connection with single-time dataflow refinement to a more general as_monotonic conversion method. The as_monotonic name reflects better the intent to upgrade plans to monotonic, but can also be used uniformly to set consolidation requirements. The commit also includes an as_monotonic method in CollationPlan, making the usage in single-time dataflow refinement more consistent.

This commit introduces the consolidate_named_if method in CollectionExt that allows us to conditionally consolidate a differential dataflow collection. The method is used in rendering in monotonic operators, which take a must_consolidate flag to indicate whether their input requires a consolidation step to turn logical into physical monotonicity. The method allows the uses in rendering to reduce code duplication and avoid mutability in collection assignments.

teskje

Still LGTM! Thanks for the readability improvements!

src/compute-client/src/plan/mod.rs

This commit adds instrumentation to finalize_dataflow so that lowering from MIR to LIR as well as subsequent plan refinements can be observed through EXPLAIN OPTIMIZER TRACE. As part of the commit, the lowering step is abstracted into a separate function and the physical plan explain stage is renamed to finalize_dataflow, where mir_to_lir is but one of its components.

ggevay

LGTM!

vmarcos · 2023-04-19T10:53:07Z

Thank you all for the reviews!

frankmcsherry force-pushed the monotonic_select branch 2 times, most recently from f2ba0d7 to 3755c60 Compare April 1, 2023 19:30

vmarcos mentioned this pull request Apr 3, 2023

Design doc for LIR-level optimizations #18535

Closed

5 tasks

vmarcos force-pushed the monotonic_select branch 2 times, most recently from 99302cb to 16933dd Compare April 5, 2023 12:05

vmarcos force-pushed the monotonic_select branch 3 times, most recently from e0f5221 to 8895d80 Compare April 6, 2023 10:18

antiguru reviewed Apr 6, 2023

View reviewed changes

src/compute-client/src/explain/text.rs Outdated Show resolved Hide resolved

src/compute-client/src/explain/text.rs Outdated Show resolved Hide resolved

src/compute-client/src/explain/text.rs Outdated Show resolved Hide resolved

vmarcos force-pushed the monotonic_select branch 2 times, most recently from b09d5c8 to 24d6932 Compare April 6, 2023 16:21

vmarcos requested review from aalexandrov, jkosh44 and teskje April 6, 2023 17:10

vmarcos marked this pull request as ready for review April 6, 2023 17:13

vmarcos requested a review from a team as a code owner April 6, 2023 17:13

vmarcos requested a review from a team April 6, 2023 17:13

jkosh44 reviewed Apr 6, 2023

View reviewed changes

src/sql/src/session/vars.rs Show resolved Hide resolved

jkosh44 approved these changes Apr 6, 2023

View reviewed changes

vmarcos mentioned this pull request Apr 10, 2023

Output buckets in physical EXPLAIN text, not skips #18690

Merged

5 tasks

vmarcos self-assigned this Apr 10, 2023

teskje approved these changes Apr 11, 2023

View reviewed changes

vmarcos force-pushed the monotonic_select branch from 24d6932 to 58e84c1 Compare April 11, 2023 15:26

This was referenced Apr 12, 2023

Improve must_consolidate setting in single-time dataflow refinement #18732

Closed

Enforce testing of both monotonic and non-monotonic plans #18734

Closed

[Epic] Specialized evaluation / optimization for interactive SELECT queries #14442

Closed

aalexandrov reviewed Apr 12, 2023

View reviewed changes

vmarcos force-pushed the monotonic_select branch from 1b5a444 to 6230505 Compare April 18, 2023 21:12

Frank McSherry and others added 9 commits April 18, 2023 22:18

Add must_consolidate to monotonic plans

adcba52

Upgrade SELECT queries to monotonic plans

81b6e38

ensure_monotonic in monotonic top-k

0115ac9

This commit adds a monotonicity check with the EnsureMonotonic operator to the rendering of monotonic top-k. This step increases the consistency of the rendering code across monotonic top-k, monotonic top-1, and monotonic hierarchical reductions.

vmarcos force-pushed the monotonic_select branch from 6230505 to a80ee03 Compare April 18, 2023 21:39

teskje approved these changes Apr 19, 2023

View reviewed changes

src/compute-client/src/plan/mod.rs Outdated Show resolved Hide resolved

src/compute-client/src/plan/mod.rs Show resolved Hide resolved

vmarcos force-pushed the monotonic_select branch from a80ee03 to 44c0888 Compare April 19, 2023 10:10

ggevay approved these changes Apr 19, 2023

View reviewed changes

vmarcos merged commit e70912c into MaterializeInc:main Apr 19, 2023

This was referenced Apr 19, 2023

Flag inputs to SELECT queries as monotonic if until is set. #14607

Closed

SELECT query expressions can be marked as append-only #14351

Closed

Evaluate in-place consolidation in monotonic one-shot SELECTs with must_consolidate = true #18890

Closed

materialize-bot mentioned this pull request Apr 21, 2023

release: v0.52.0 required reviews #18907

Closed

12 tasks

aalexandrov mentioned this pull request May 9, 2023

EXPLAIN pipeline should reflect different optimization paths #18089

Closed

7 tasks

vmarcos mentioned this pull request May 15, 2023

Monotonic one-shot SELECT feature fails to get activated in serializable mode with source that emits the empty frontier #19290

Closed

vmarcos mentioned this pull request May 24, 2023

Exploration of hashing approaches in consolidate_named_if #19372

Merged

5 tasks

vmarcos mentioned this pull request Jul 18, 2023

Single-time consolidation refinement #19680

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monotonic select #18546

Monotonic select #18546

frankmcsherry commented Apr 1, 2023

CLAassistant commented Apr 1, 2023 •

edited

Loading

vmarcos commented Apr 3, 2023

vmarcos commented Apr 4, 2023 •

edited

Loading

vmarcos commented Apr 5, 2023

antiguru left a comment

vmarcos commented Apr 6, 2023

jkosh44 left a comment

vmarcos commented Apr 10, 2023

frankmcsherry commented Apr 10, 2023

teskje left a comment

teskje Apr 11, 2023

vmarcos Apr 11, 2023

aalexandrov left a comment

aalexandrov Apr 12, 2023

vmarcos Apr 13, 2023

aalexandrov Apr 12, 2023

vmarcos Apr 13, 2023

teskje left a comment

ggevay left a comment

vmarcos commented Apr 19, 2023

Monotonic select #18546

Monotonic select #18546

Conversation

frankmcsherry commented Apr 1, 2023

Motivation

Tips for reviewer

Checklist

CLAassistant commented Apr 1, 2023 • edited Loading

vmarcos commented Apr 3, 2023

vmarcos commented Apr 4, 2023 • edited Loading

vmarcos commented Apr 5, 2023

antiguru left a comment

Choose a reason for hiding this comment

vmarcos commented Apr 6, 2023

jkosh44 left a comment

Choose a reason for hiding this comment

vmarcos commented Apr 10, 2023

frankmcsherry commented Apr 10, 2023

teskje left a comment

Choose a reason for hiding this comment

teskje Apr 11, 2023

Choose a reason for hiding this comment

vmarcos Apr 11, 2023

Choose a reason for hiding this comment

aalexandrov left a comment

Choose a reason for hiding this comment

aalexandrov Apr 12, 2023

Choose a reason for hiding this comment

vmarcos Apr 13, 2023

Choose a reason for hiding this comment

aalexandrov Apr 12, 2023

Choose a reason for hiding this comment

vmarcos Apr 13, 2023

Choose a reason for hiding this comment

teskje left a comment

Choose a reason for hiding this comment

ggevay left a comment

Choose a reason for hiding this comment

vmarcos commented Apr 19, 2023

CLAassistant commented Apr 1, 2023 •

edited

Loading

vmarcos commented Apr 4, 2023 •

edited

Loading