fix: Enhance case expression type coercion

[Jefffrey]

Which issue does this PR close?

Closes #5538

Closes #5828

Rationale for this change

Given following:

SELECT
  CASE a
    WHEN b THEN 0
    WHEN c THEN 1
    ELSE 2
  END;

Should attempt to coerce a, b and c to common type.

And for:

SELECT
  CASE
    WHEN a THEN 0
    WHEN b THEN 1
    ELSE 2
  END;

Should coerce a and b to boolean type.

What changes are included in this PR?

Ensure case and when expressions are coerced to common type, similar to how currently then and else expressions are coerced to common type.

Also ensure when expressions for when case expression isn't provided to coerce to boolean type.

Are these changes tested?

Added to sqllogictest

Are there any user-facing changes?

[alamb]

Thank you @Jefffrey -- the code looks good to me

Prior to merge I think some additional tests are needed in this PR.

I also left some small code style suggestions, but I don't think they are required.

[alamb]

❤️

[alamb]

You can write this more concisely / functionally using transpose like this if you want

Suggested change

	// prepare types
	let case_type = match &case.expr {
	None => Ok(None),
	Some(expr) => expr.get_type(&self.schema).map(Some),
	}?;
	// prepare types
	let case_type = case.expr.as_ref()
	.map(|expr| expr.get_type(&self.schema))
	.transpose()?;

The same basic transformation applies to the other match statements below -- I don't think it is a big deal I just figured I would point it out to you

[Jefffrey]

Thanks that does look cleaner, will apply it

[alamb]

Could you add some additional tests?

1. Alternate Syntax:

select CASE 
  WHEN 10 = 5 THEN 1 
  WHEN 'true' THEN 1 
  ELSE 10 
END as col;

Also negative cases like

select CASE 10.5 WHEN 'not a number' THEN 1 ELSE 10 END as col;

select CASE 
  WHEN 10 = 5 THEN 1 
  WHEN 'not boolean' THEN 1 
  ELSE 10 
END as col;

select CASE 
  WHEN 10 = 5 THEN 1 
  WHEN 5 THEN 'not a number'
  ELSE 10 
END as col;

select CASE 
  WHEN 10 = 5 THEN 1 
  WHEN 5 THEN 0
  ELSE 'goo' 
END as col;

[Jefffrey]

Is there a way in sqllogictests to change the config of DataFusion? Since for negative cases to produce error, would need to set datafusion.optimizer.skip_failed_rules to false, unless you mean for the negative case to fail as in the original issue?

Otherwise can add the negative case as a unit test in actual code

[alamb]

Yes, you can use the SET command to change config values. For example https://github.com/apache/arrow-datafusion/blob/7b90474adf6c1afbb4a5988a5677834b07347321/datafusion/core/tests/sqllogictests/test_files/information_schema.slt#L30

[Jefffrey]

Testing with sqllogictests turned out to be kinda finicky, as this PR is meant to enhance the type coercion, but not actually testing the actual casting itself (since some of those negative cases would pass the type coercion but fail at execution time due to actual contents of the data). So I figured it'd be easier to add more tests as unit tests for proper testing.

[Jefffrey]

I've found another bug related to case expression casting, will raise issue and also fix as part of this PR since will be affecting similar code

Edit: the bug #5828

Edit2: fix added

[alamb]

I just think we need a few more tests and this PR will be ready to go

[alamb]

🤔

[Jefffrey]

Yeah figured better to wrap in Context to hopefully provide more informative error message

[alamb]

Looks great to me -- thank you @Jefffrey

[alamb]

I found this code really easy to read. Thank you @Jefffrey