fix: correct expected error in test #5224
Conversation
|
cc @HaoYang670 |
jackwener
changed the title
cast_string_to_time
|
This PR must be with disable skip config |
|
async fn cast_string_to_time() {
let config = SessionConfig::new().set("datafusion.optimizer.skip_failed_rules", ScalarValue::Boolean(Some(false)));
let ctx = SessionContext::with_config(config); |
It's great idea. thank you! @HaoYang670 |
jackwener
force-pushed
the
fix_comment
branch
from
3122f93 to
cef3735
Compare
| r#"type_coercion | ||
| caused by | ||
| Internal error: Optimizer rule 'type_coercion' failed due to unexpected error: Error during planning: Can not find compatible types to compare Boolean with [Struct([Field { name: "foo", data_type: Boolean, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }]), Utf8]. This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker"# | ||
| ); |
There was a problem hiding this comment.
We should update this error message somehow, because it is a type error, not a bug in Datafusion.
We could do this in the following PRs.
There was a problem hiding this comment.
Agree with you.
I think we can do it in next PR.
| "Arrow error: Cast error: Cannot cast string 'not a time' to value of Time64(Nanosecond) type" | ||
| "simplify_expressions\ncaused by\nInternal error: Optimizer rule 'simplify_expressions' failed due to unexpected error: \ | ||
| Arrow error: Cast error: Cannot cast string 'not a time' to value of Time64(Nanosecond) type. \ | ||
| This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker" |
There was a problem hiding this comment.
Ditto. We should find a way to update the error message when the error is expected. However, this is not a big deal now, because the default value of skip_failed_rules is false and customers won't see this error message.
| ); | ||
|
|
||
| // An invalid time | ||
| let sql = "SELECT TIME '24:01:02' as time;"; | ||
| let result = try_execute_to_batches(&ctx, sql).await; | ||
| assert_eq!( | ||
| result.err().unwrap().to_string(), | ||
| "Arrow error: Cast error: Cannot cast string '24:01:02' to value of Time64(Nanosecond) type" | ||
| "simplify_expressions\ncaused by\nInternal error: Optimizer rule 'simplify_expressions' failed due to unexpected error: \ |
|
Benchmark runs are scheduled for baseline = a1b6f50 and contender = 8a262c3. 8a262c3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #5217.
Part of #4685
Rationale for this change
What changes are included in this PR?
No need, current tests can cover this problem, but it was hidden by skip_failed_rules.
set
pub skip_failed_rules: bool, default = falseBefore this PR,
cast_string_to_timeandin_list_types_struct_literalwill fail.After this PR, they will success.
Are these changes tested?
Are there any user-facing changes?