Add reproducer for tpch Q16 deserialization bug #16662
Conversation
| let err = | ||
| result.expect_err("Expected deserialization to fail due to type mismatch bug"); | ||
| assert!( | ||
| err.to_string().contains("inlist should be same") | ||
| && err.to_string().contains("Int64") | ||
| && err.to_string().contains("Int32"), | ||
| "Expected type mismatch error with Int64/Int32, got: {}", | ||
| err | ||
| ); |
There was a problem hiding this comment.
Other option could be to instead of expecting the current error, write the test with the normal expectations (it succeeding), and mark it as #[ignore] with a TODO comment just above the test name.
Just giving options, don't have a strong preference, if you think it's fine as it is right now lets go with that 👍
There was a problem hiding this comment.
I agree it would be better to mark this test as #[ignore] as that follows the patterns of the rest of this repo more closely
There was a problem hiding this comment.
I have made the test failed and ignore it. Thanks @gabotechs and @alamb
| let err = | ||
| result.expect_err("Expected deserialization to fail due to type mismatch bug"); | ||
| assert!( | ||
| err.to_string().contains("inlist should be same") | ||
| && err.to_string().contains("Int64") | ||
| && err.to_string().contains("Int32"), | ||
| "Expected type mismatch error with Int64/Int32, got: {}", | ||
| err | ||
| ); |
There was a problem hiding this comment.
I agree it would be better to mark this test as #[ignore] as that follows the patterns of the rest of this repo more closely
|
Thanks @NGA-TRAN and @gabotechs |
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @NGA-TRAN and @gabotechs
I also wonder if we should add examples / tpch serialization for all the remaining tpch queries in datafuison-proto 🤔
The clippy failure seems to be some sort of network issue unrelated to this PR
|
Actually there was a real clippy bug but I pushed a fix as I had this PR open anyways locally |
Good idea. I did this in the distributed repro but I will add them here (in a different PR) |
|
Thanks again @NGA-TRAN and @gabotechs |
3 participants
Which issue does this PR close?
Reproducing a bug
Rationale for this change
Datadog is working on building a distributed version of DataFusion, which requires query serialization and deserialization. While testing with TPC-H queries, we found that Q16 fails during deserialization—potentially due to an issue in the serialization step. I've minimized the query to a smaller form that still reproduces the problem.
What changes are included in this PR?
Adding a test
Are these changes tested?
This is not a code change. It is a test to reproduce a bug
Are there any user-facing changes?
No