zclllyybb commented on issue #64179: URL: https://github.com/apache/doris/issues/64179#issuecomment-4642234543
Breakwater-GitHub-Analysis-Slot: slot_b8bdf6bb8446 I rechecked the current upstream `origin/master` code, and this looks like a real BE bug rather than only a test artifact. Evidence: - `VExpr::VExpr(DataTypePtr, bool)` initializes `_opcode` and `_data_type`, but only assigns `_node_type` when `is_slotref` is true. - `_node_type` has no in-class default initializer, while `is_slot_ref()` directly checks `_node_type == TExprNodeType::SLOT_REF`. - The false branch is reachable in current code: for example `VirtualSlotRef(const SlotDescriptor*)` calls `VExpr(desc->type(), false)`, and several BE tests also construct non-slot expressions through this constructor. So the reported crash chain is consistent: a non-`VSlotRef` expression can be misclassified as a slot ref if the uninitialized value happens to equal `SLOT_REF`, and a later `assert_cast<VSlotRef*>` then fails. There is already an in-flight fix PR, #64180. One review caveat: the current patch uses `TExprNodeType::INVALID_OPCODE` as the fallback node type, but current `gensrc/thrift/Exprs.thrift` does not define that enum value. `INVALID_OPCODE` belongs to `TExprOpcode`, not `TExprNodeType`, so that part likely needs adjustment before the patch can compile. Suggested next step: - Initialize `_node_type` on all constructor paths using a valid `TExprNodeType` value, or refactor this constructor to receive the actual expression node type instead of only a boolean. - For descriptor-backed `VirtualSlotRef`, preserving `TExprNodeType::VIRTUAL_SLOT_REF` would be more semantically precise than using an arbitrary non-slot placeholder. - Keep/add a regression test that constructs a non-slot `VExpr(type, false)` and verifies it is not treated as a slot ref. No extra runtime logs are needed to confirm the root cause. The only useful missing context is the exact source branch or PR containing `RuntimeFilterPartitionPrunerTest.ProjectedBoundariesPreserveOpenRangeBounds`, since that test symbol is not present in the current fetched `origin/master` tree. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
