[SQL] Improved unused field analysis: proper dataflow analysis, and less conservative handling of nullable tuples#5826
Conversation
mythical-fred
left a comment
There was a problem hiding this comment.
LGTM. Tracking used fields (positive) is cleaner than tracking unused (negative), and the FieldSet union correctly handles t.1.is_null() on nullable tuples without touching any inner fields.
| FieldUseMap fieldMap1 = fum.getUse(); | ||
| Assert.assertEquals("Ref([X, _, _, [_, _]])", fieldMap1.toString()); | ||
|
|
||
| FieldUseMap reduced = fieldMap0.reduce(fieldMap1); |
There was a problem hiding this comment.
Non-blocking: the fix for #5823 (t.1.is_null() on a nullable tuple should not mark inner fields as used) would be worth a small unit test here. The regression currently lives only in the external QA repo. Not blocking.
|
The CI failure is an interesting one: there is a test which checks that when adding a new view, the existing program does not change. Well, in this case a table had unused fields, but the newly added view uses these fields, so the new view DOES modify the existing program. So I will have to come up with a different test... |
…ess conservative handling of nullable tuples Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
Fixes #5823
Describe Manual Test Plan
I have tested with a new program that I am adding to the qa repository that this analysis is more precise.