Monotonic select by frankmcsherry · Pull Request #18546 · MaterializeInc/materialize

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 MaterializeInc/database-issues#4838. 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, MaterializeInc/database-issues#1) }                                            +
       Mfp                                                                                                          +
         project=(#0, MaterializeInc/database-issues#1)                                                                                           +
         map=(null, 0)                                                                                              +
         Union                                                                                                      +
           Negate                                                                                                   +
             Get::Arrangement l0                                                                                    +
               project=()                                                                                           +
               key=                                                                                                 +
               raw=false                                                                                            +
               arrangements[0]={ key=[], permutation=id, thinning=(#0, MaterializeInc/database-issues#1) }                                        +
           Constant                                                                                                 +
             - ()                                                                                                   +
   With                                                                                                             +
     cte l0 =                                                                                                       +
       Reduce::Accumulable                                                                                          +
         simple_aggrs[0]=(0, 0, sum(#0))                                                                            +
         simple_aggrs[1]=(1, 1, count(*))                                                                           +
         val_plan                                                                                                   +
           project=(#0, MaterializeInc/database-issues#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 - MaterializeInc/database-issues#2))))                                                         +
               val_plan                                                                                             +
                 project=(#3)                                                                                       +
                 map=((#1 * (1 - MaterializeInc/database-issues#2)))                                                                              +
               key_plan                                                                                             +
                 project=(#0)                                                                                       +
               Get::Arrangement tpch.public.lineitem                                                                +
                 project=(#0, MaterializeInc/materialize#5, MaterializeInc/database-issues#5)                                                                               +
                 key=#0, MaterializeInc/database-issues#3                                                                                         +
                 raw=false                                                                                          +
                 arrangements[0]={ key=[#0, MaterializeInc/database-issues#3], permutation={#1: MaterializeInc/database-issues#2, MaterializeInc/database-issues#2: MaterializeInc/database-issues#3, MaterializeInc/database-issues#3: MaterializeInc/database-issues#1}, thinning=(#1, MaterializeInc/database-issues#2, MaterializeInc/database-issues#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!

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 https://github.com/MaterializeInc/database-issues/issues/5301.
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!

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

+                    dataflow.until.join_assign(&sink.up_to);
                }
            }
+            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 https://github.com/MaterializeInc/materialize/issues/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.

aalexandrov

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

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

+                    }
+
+                    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.

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

-                    .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 https://github.com/MaterializeInc/materialize/issues/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!

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!

…40061) This reverts #40029, because its in-container restart loop lets a fenced environmentd reboot inside a pod that Kubernetes has already replaced, take a new catalog epoch and fence out the live leader. ### Motivation The nightly job `K8s recovery: envd on failing node` fails on main since #40029 ([SQL-807](https://linear.app/materializeinc/issue/SQL-807)). * Failing: [nightly #18553](https://buildkite.com/materialize/nightly/builds/18553#01a15ba6-bf60-4bb3-b6ff-db71cf56c89a) (main, all three attempts), [#18550](https://buildkite.com/materialize/nightly/builds/18550) and [#18551](https://buildkite.com/materialize/nightly/builds/18551) (branches that contain #40029). * Passing: every nightly without #40029 ([#18543](https://buildkite.com/materialize/nightly/builds/18543), [#18545](https://buildkite.com/materialize/nightly/builds/18545), [#18546](https://buildkite.com/materialize/nightly/builds/18546), [#18548](https://buildkite.com/materialize/nightly/builds/18548), [#18552](https://buildkite.com/materialize/nightly/builds/18552), [#18556](https://buildkite.com/materialize/nightly/builds/18556)). One branch build with #40029, [#18554](https://buildkite.com/materialize/nightly/builds/18554), passed, so the failure is a race and not deterministic. * Signature: `Failed to achieve 'no issues after node recovery' ... within 10s`. The replacement `environmentd-0` panics with `unable to advance catalog upper: Durable(Fence(Epoch { current_epoch: 3, fence_epoch: 4 }))` (exit 134) right after the old envd node is unpaused. Timeline from #18553 attempt 3 (kail, clusterd logs, k8s events): * 00:53:35 UTC: the original environmentd boots on `mzcloud-worker9` (pod IP 10.244.5.2) and initializes the catalog (epoch 2). 00:54:22: the test pauses worker9. * 01:00:14.6: the replacement `environmentd-0` boots on worker8 (10.244.2.2), opens the catalog writable (epoch 3) and serves queries. * 01:00:20.41: the test unpauses worker9. * 01:00:20.98: clusterds accept new connections from 10.244.5.2 and drop the replacement's. * 01:00:21.28: the replacement panics on the fence with `fence_epoch: 4`. * 01:00:21.44: the 10.244.5.2 connections close; a `Killing` event (`Stopping container environmentd`) follows. The replacement restarts and the test query hits it. Only a writable catalog open bumps the epoch (`Mode::Writable` in `src/catalog/src/durable/persist.rs`), and the replacement booted once in that window. So epoch 4 came from a new environmentd process started inside the old, already deleted pod on worker9. The events show no second `Started` for the old pod; its only container start is the original one at 00:53. That fits the wrapper's new behavior: * The resumed old environmentd is fenced and halts with exit code 166, for example via `fenced by envd` in `storage_collections.rs` or the txn-wal fence. * Before #40029, `entrypoint.sh` exited the container on 166. For a pod that is already deleted, the kubelet does not restart it. * With #40029, the wrapper restarts environmentd immediately. The new process opens the catalog, takes the next epoch and fences the legitimate leader until the kubelet kills the container. The same can happen outside the test whenever an environmentd that lost leadership halts while its pod is being replaced, for example after a node failure. The restart loop bypasses the kubelet's decision about whether that container should run again. ### Description `git revert e6e3e21`: restore `tini` without `-g` and the `entrypoint.sh` that exits on any nonzero status. Avoiding crash backoff for repeated catch-up halts during zero-downtime upgrades ([SQL-807](https://linear.app/materializeinc/issue/SQL-807)) still needs a fix. A restart that only covers halts that are safe to retry in place, such as a distinct exit code for the preflight reboots in `src/environmentd/src/deployment/preflight.rs`, would avoid this. That is left to the owner. ### Tests * No local run: reproducing it needs the cloudtest kind cluster and a full image build. * Please run `K8s recovery: envd on failing node` from the nightly pipeline against this branch before merging. Since the failure is a race, a pass is only meaningful together with the three failing attempts on #18553. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>

) This re-lands the in-container restart of #40029 ([SQL-807](https://linear.app/materializeinc/issue/SQL-807)) for the read-only catch-up halts only, so a fenced environmentd in a replaced pod no longer restarts and fences the live leader. ### Motivation #40029 restarted environmentd inside its container on every `halt!` (exit 166), to avoid Kubernetes crash backoff for repeated catch-up halts during zero-downtime upgrades. #40061 reverted it, and this PR is based on main after that revert, because #40029 broke the nightly job `K8s recovery: envd on failing node`. * Failing with #40029: [nightly #18553](https://buildkite.com/materialize/nightly/builds/18553#01a15ba6-bf60-4bb3-b6ff-db71cf56c89a) (main, all three attempts), [#18550](https://buildkite.com/materialize/nightly/builds/18550), [#18551](https://buildkite.com/materialize/nightly/builds/18551). One branch build with #40029, [#18554](https://buildkite.com/materialize/nightly/builds/18554), passed, so the failure is a race. * Passing without it: [#18543](https://buildkite.com/materialize/nightly/builds/18543), [#18545](https://buildkite.com/materialize/nightly/builds/18545), [#18546](https://buildkite.com/materialize/nightly/builds/18546), [#18548](https://buildkite.com/materialize/nightly/builds/18548), [#18552](https://buildkite.com/materialize/nightly/builds/18552), [#18556](https://buildkite.com/materialize/nightly/builds/18556). * Signature: the replacement `environmentd-0` panics with `unable to advance catalog upper: Durable(Fence(Epoch { current_epoch: 3, fence_epoch: 4 }))` right after the old envd node is unpaused. * Mechanism: the old environmentd, resumed after its pod was replaced, is fenced and halts with 166. The kubelet would not restart a deleted pod, but the #40029 wrapper did. The new process opened the catalog writable, took epoch 4 and fenced the replacement until the kubelet killed the container. The halts that [SQL-807](https://linear.app/materializeinc/issue/SQL-807) is about are the DDL reboots in `check_ddl_changes`, which a read-only deployment can hit many times while it catches up. ### Description * `mz_ore::process`: add `RESTART_IN_PLACE_EXIT_CODE` (75, `EX_TEMPFAIL`) and the `restart_in_place!` macro, which behaves like `halt!` but exits with 75. Name the existing halt code `HALT_EXIT_CODE` (166). Shells, tini and Kubernetes report death by signal N as 128 + N, so a code below 128 can only come from an explicit exit. * `src/environmentd/ci/entrypoint.sh`: restart environmentd in place only on 75, and export `MZ_RESTARTED_IN_PLACE=true` before doing so. Every other exit, including 166, ends the container as before #40029. A graceful exit still sleeps forever. * `preflight_0dt`: when `MZ_RESTARTED_IN_PLACE` is set (the hidden `--restarted-in-place` flag) and the catalog is at this process's own generation, halt with 166 instead of booting writable ([`preflight.rs#L70-L82`](https://github.com/MaterializeInc/materialize/blob/a5f2ab0c41e844fc109211a460fb76a53c1282ca/src/environmentd/src/deployment/preflight.rs#L70-L82)). * Only read-only code paths restart in place, so that combination means a process of this generation has been promoted: one in a pod that replaced this one, or, after a skipped catch-up, this container's previous process, whose read-only bootstrap restarted after the takeover. Booting writable would fence a replacement, and halting is correct in both cases. * A kubelet restart starts a new container without the marker, so a pod that wasn't replaced still boots as leader. * `check_ddl_changes`: use `restart_in_place!` only while the catalog's deployment generation is still older than this process's, so the restarted process boots read-only. Once a process of this generation has been promoted, halt with 166 (`deployment generation N has been promoted; halting`). The generation is read from the openable catalog that the check already builds, before `open_savepoint` consumes it, so the check reads one catalog snapshot, not two. * Storage controller: the read-only halt in `create_collections_for_bootstrap` for a dependency deleted during bootstrap uses `restart_in_place!` ([`lib.rs#L1031-L1047`](https://github.com/MaterializeInc/materialize/blob/a5f2ab0c41e844fc109211a460fb76a53c1282ca/src/storage-controller/src/lib.rs#L1031-L1047)). It keeps `halt!` when writable. * The takeover halt after promotion (`fenced out old deployment; rebooting as leader`) keeps `halt!`. That process has just opened the catalog writable, and a restart in place would only reach the marker check one boot later. * `tini -g` and the entrypoint's TERM/INT traps are restored as #40029 had them: environmentd receives SIGTERM itself, and the shell exits after it instead of restarting or sleeping. * `bin/environmentd` (`misc/python/materialize/cli/run.py`) restarts on 75 as it does on 166. Residual: a stale read-only process in a replaced pod still restarts in place on these halts until the kubelet kills its container. The marker keeps it from booting writable, but it competes with the replacement for the generation's replicas during that window. ### Tests * `cargo check` and `cargo clippy --all-targets -- -D warnings` for `mz-ore`, `mz-environmentd`, `mz-storage-controller` and `mz-sqllogictest` with Rust 1.98.1: clean. `rustfmt --check` on the changed Rust files: clean. * `shellcheck -s sh src/environmentd/ci/entrypoint.sh` (koalaman/shellcheck): clean. * Entrypoint loop under busybox `sh` with a stub `environmentd`: * exits 75 then 0: restarted once with the same arguments and `MZ_RESTARTED_IN_PLACE=true`, then sleeps. * exits 75, 75, 166: restarted twice, then the container exits 166. * exits 166, 1 or 134: the container exits with that code, no restart. * `docker stop` under tini with `-g`: environmentd gets SIGTERM, and the container exits 143 within 0.1 s. * `test/0dt` workflow `restarted-in-place-after-promotion`: promotes `mz_new`, then starts a second process of its generation with `MZ_RESTARTED_IN_PLACE=true`, and asserts that it exits 166 with the marker halt while `mz_new` keeps serving as leader. Without the marker check, that process would fence `mz_new`. * Not run: `K8s recovery: envd on failing node` needs the cloudtest kind cluster, and it and the `test/0dt` workflows, including the new one, need image builds. Please run the nightly job against this branch before merging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>

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

Comment thread src/compute-client/src/explain/text.rs Resolved

Comment thread src/compute-client/src/explain/text.rs Resolved

Comment thread src/compute-client/src/explain/text.rs 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

Comment thread src/sql/src/session/vars.rs 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

3 of 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

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

Comment thread src/compute-client/src/plan/mod.rs Resolved

Comment thread src/compute-client/src/plan/mod.rs 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

vmarcos mentioned this pull request Apr 19, 2023

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

Closed

3 tasks

vmarcos mentioned this pull request May 24, 2023

Exploration of hashing approaches in consolidate_named_if #19372

Merged

2 of 5 tasks

vmarcos mentioned this pull request Jul 18, 2023

Single-time consolidation refinement #19680

Merged

5 tasks done

Conversation

frankmcsherry commented Apr 1, 2023

Motivation

Tips for reviewer

Checklist

Uh oh!

CLAassistant commented Apr 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vmarcos commented Apr 3, 2023

Uh oh!

vmarcos commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vmarcos commented Apr 5, 2023

Uh oh!

antiguru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vmarcos commented Apr 6, 2023

Uh oh!

Uh oh!

jkosh44 left a comment

Choose a reason for hiding this comment

Uh oh!

vmarcos commented Apr 10, 2023

Uh oh!

frankmcsherry commented Apr 10, 2023

Uh oh!

teskje left a comment

Choose a reason for hiding this comment

Uh oh!

teskje Apr 11, 2023

Choose a reason for hiding this comment

Uh oh!

vmarcos Apr 11, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aalexandrov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aalexandrov Apr 12, 2023

Choose a reason for hiding this comment

Uh oh!

vmarcos Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aalexandrov Apr 12, 2023

Choose a reason for hiding this comment

Uh oh!

vmarcos Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

teskje left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ggevay left a comment

Choose a reason for hiding this comment

Uh oh!

vmarcos commented Apr 19, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

CLAassistant commented Apr 1, 2023 •

edited

Loading

vmarcos commented Apr 4, 2023 •

edited

Loading