feat: Add a test for supported types of SortMergeJoin in DataFusion #365
Conversation
| @@ -383,6 +383,13 @@ object CometConf { | |||
| .booleanConf | |||
| .createWithDefault(false) | |||
|
|
|||
| val COMET_SORTMERGEJOIN_CHECK_KEY_TYPES: ConfigEntry[Boolean] = | |||
There was a problem hiding this comment.
thanks @planga82
I'm not sure if we need to change the conf for the unit test purpose
There was a problem hiding this comment.
thanks @comphead . Any idea how to do this in a cleaner way? We could move this constant to other place and avoid putting it in CometConf. In that way we avoid to expose a testing configuration. What do you think?
There was a problem hiding this comment.
Tbh to test variety of supported datatypes I liked the @andygrove approach introduced in #351, wondering if we can do that
There was a problem hiding this comment.
I'm going to review it. Thanks!
There was a problem hiding this comment.
I think it's not the same idea and not applicable here. I'll close this PR and try to think of another way to do it without adding configuration. Thanks again!
Which issue does this PR close?
Closes #357 .
Rationale for this change
The goal of these tests is that if Datafusion supports more data types, we remember to support them here.
What changes are included in this PR?
Add new tests
How are these changes tested?
Unit testing
We have yet configuration to support testing (COMET_EXEC_BROADCAST_FORCE_ENABLED) so this PR add new configuration to support the tests.
I can't do complex type testing because we don't support it in most operators.