Files
gristlabs_grist-core/sandbox/grist
Alex Hall d154b9afa7 (core) Make lookups depend on all rows
Summary:
This is a fix for a bug discussed in https://grist.slack.com/archives/C069RUP71/p1645138610722889

I still haven't completely wrapped my head around it or figured out how to make a simple reproducible example, but the problem seems to be that a lookup can happen before the column(s) being looked up (the summary helper column in this case) have been computed fully (I think it got interrupted halfway by an OrderError). `do_lookup` would check via `engine._use_node` that the row IDs it found had all been computed already, but there might still be other rows that hadn't been computed yet and would also have values matching the lookup key, so it missed those.

This diff instead calls `_use_node` with no `row_ids` argument, which should ensure that all rows have already been computed.

At first I was worried about how this would affect performance, which led me down an optimisation rabbit hole, hence a bit of unrelated cleanup here and also https://phab.getgrist.com/D3310 . But it doesn't seem to be a problem, and IIUC it should actually make things better, although this code is pretty confusing.

Test Plan: Tested manually that the doc no longer behaves weirdly

Reviewers: dsagal

Reviewed By: dsagal

Subscribers: dsagal, paulfitz

Differential Revision: https://phab.getgrist.com/D3308
2022-03-14 19:42:51 +02:00
..
2022-03-08 12:14:39 +02:00
2022-02-19 09:46:49 +00:00
2022-03-08 12:14:39 +02:00
2020-07-29 08:57:25 -04:00
2022-02-19 09:46:49 +00:00
2022-03-08 12:14:39 +02:00
2022-02-19 09:46:49 +00:00
2022-02-19 09:46:49 +00:00