materialize
materialize copied to clipboard
Fix visitation in `HirRelationExpr::contains_temporal`
The problem was that contains_temporal was just calling visit_children, which doesn't do a visitation in the entire subtree, but merely visits the scalar expressions in the top-level relation expr node. This PR changes contains_temporal to do a visitation of all relation expr nodes, and for each of them do what we were doing before for just the top-level node.
The issue description mentions that we should be able to find temporal expressions also in referenced view definitions. This already works: peek_stage_validate calls validate_timeline_context, which calls get_timeline_contexts, which transitively visits view definitions and calls MirRelationExpr::contains_temporal() on their optimized expressions.
Motivation
- This PR partially fixes a recognized bug: https://github.com/MaterializeInc/materialize/issues/25339
- It doesn't fix it for
could_run_expensive_function.
- It doesn't fix it for
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.
- [ ] If this PR evolves an existing
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel. - [ ] If this PR will require changes to cloud orchestration or tests, 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: