Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Enable additional CreateArray tests #928

Merged
merged 2 commits into from
Sep 27, 2024

Conversation

Kimahriman
Copy link
Contributor

Which issue does this PR close?

Follow up to #792

Rationale for this change

Enable additional CreateArray tests now that we've bumped the DataFusion version in #909

What changes are included in this PR?

Enable additional tests, and make on change to the equality check for type coercion to fix one of the new tests.

How are these changes tested?

New tests enabled

@@ -1855,7 +1855,7 @@ impl PhysicalPlanner {
.into_iter()
.zip(input_expr_types.into_iter().zip(coerced_input_types))
.map(|(expr, (from_type, to_type))| {
if !from_type.equals_datatype(&to_type) {
if from_type != to_type {
Copy link
Contributor Author

@Kimahriman Kimahriman Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MutableArrayData checks for exact equality across types, including field names, so need to be stricter about deciding when casts are needed.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.04%. Comparing base (033fe6f) to head (bd02660).
Report is 14 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #928      +/-   ##
============================================
+ Coverage     34.03%   34.04%   +0.01%     
+ Complexity      883      882       -1     
============================================
  Files           113      112       -1     
  Lines         43170    43290     +120     
  Branches       9516     9553      +37     
============================================
+ Hits          14693    14739      +46     
- Misses        25471    25537      +66     
- Partials       3006     3014       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Kimahriman

@andygrove andygrove merged commit 0ee7df8 into apache:main Sep 27, 2024
75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants