koopatroopa787 commented on PR #22464: URL: https://github.com/apache/datafusion/pull/22464#issuecomment-4535421517
Alright so after doing another review I realized the issue with roundtrip. It only called try_from_proto on a hand-built node, never going through try_to_proto. Rewrote the whole test module to follow the like.rs pattern: try_to_proto_encodes_unknown_column — calls try_to_proto, destructures the result, checks the name is encoded correctly and expr_id is None try_from_proto_decodes_name — builds a proto node by hand and verifies decoding try_from_proto_rejects_non_unknown_column_node — passes a wrong-type node, checks the error is DataFusionError::Internal with the actual expr_type in the message unknown_column_proto_roundtrip — the actual roundtrip, try_to_proto output fed directly into try_from_proto Also replaced the hand-rolled DummyDecode with UnreachableDecoder and StubEncoder from crate::proto_test_util, and moved the module to #[cfg(all(test, feature = "proto"))] mod proto_tests to match like.rs. On the imports: use super::* only brings in items defined in the parent module, not private use aliases like Arc, Schema, Result, internal_err. The new proto_tests module imports everything explicitly now anyway, so it's consistent with like.rs regardless. -- 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]
