Add the predicate in physical plan explain output - #22984
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe streaming explain formatter now renders predicates for conditional joins, filters, and scans, and the serializer matches ChangesStreaming explain predicate rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| _BinaryOperator.EQUAL: "==", | ||
| _BinaryOperator.NOT_EQUAL: "!=", | ||
| _BinaryOperator.LESS: "<", | ||
| _BinaryOperator.LESS_EQUAL: "<=", | ||
| _BinaryOperator.GREATER: ">", | ||
| _BinaryOperator.GREATER_EQUAL: ">=", | ||
| _BinaryOperator.LOGICAL_AND: "&", | ||
| _BinaryOperator.NULL_LOGICAL_AND: "&", | ||
| _BinaryOperator.LOGICAL_OR: "|", | ||
| _BinaryOperator.NULL_LOGICAL_OR: "|", |
There was a problem hiding this comment.
optional: Would be cool to make e.g. str(plc.binaryop.BinaryOperator.EQUAL) return "==".
There was a problem hiding this comment.
Yeah, it does look nice. But I wonder if it's important not to lose the information already provided like that it's a value from an enum?
In [4]: str(plc.binaryop.BinaryOperator.EQUAL)
Out[4]: '<binary_operator.EQUAL: 21>'
There was a problem hiding this comment.
Making __repr__ preserve this information while __str__ representing the symbolic version of the comparison could be a distinction (IIRC I think e.g. operator.eq does something similar), but overall NBD
|
/merge |
Includes the predicate in the physical plan explain output for `ConditionalJoin`, `Filter`, and `Scan`. Purely an improvement; helps make studying plan easier. Authors: - Matthew Murray (https://github.com/Matt711) Approvers: - Matthew Roeschke (https://github.com/mroeschke) URL: NVIDIA#22984
…3050) Oversight by me in #22984 Authors: - Matthew Murray (https://github.com/Matt711) Approvers: - Richard (Rick) Zamora (https://github.com/rjzamora) URL: #23050
Description
Includes the predicate in the physical plan explain output for
ConditionalJoin,Filter, andScan. Purely an improvement; helps make studying plan easier.Checklist