Skip to content

Conversation

@zhuqi-lucas
Copy link
Contributor

…ult values

Which issue does this PR close?

Part of

Closes #1789

We should fix map_values when we use native comet again.

Rationale for this change

Now we fall back to Spark for this query in #1799 , we can enable the map_values testing.

What changes are included in this PR?

Now we fall back to Spark for this query in #1799 , we can enable the map_values testing.

How are these changes tested?

@zhuqi-lucas zhuqi-lucas force-pushed the enable_map_values_testing branch from 88d454c to 0e6fd39 Compare May 29, 2025 15:45
@zhuqi-lucas zhuqi-lucas changed the title enable map_values testing since we fall back on nested types for defa… chore: enable map_values testing since we fall back on nested types for defa… May 29, 2025
@zhuqi-lucas
Copy link
Contributor Author

zhuqi-lucas commented May 29, 2025

Thank you @mbutrovich for review, i just noticed some fix in apache datafusion is in progress.
apache/datafusion#16203

But it seems we already fall back to spark map functions, so i enable the test now. After we change to comet execution, we can also keep it to see if it's fixed.

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.79%. Comparing base (f09f8af) to head (3bce824).
Report is 225 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1813      +/-   ##
============================================
- Coverage     56.12%   54.79%   -1.34%     
- Complexity      976     1002      +26     
============================================
  Files           119      129      +10     
  Lines         11743    12508     +765     
  Branches       2251     2350      +99     
============================================
+ Hits           6591     6854     +263     
- Misses         4012     4447     +435     
- Partials       1140     1207      +67     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@comphead
Copy link
Contributor

@zhuqi-lucas The problem is not in map_values it is deeper problem with reader itself, I provided some investigation details in #1789 (comment)

@comphead comphead marked this pull request as draft May 30, 2025 00:04
@zhuqi-lucas
Copy link
Contributor Author

Thank you @comphead for explain!

@comphead
Copy link
Contributor

comphead commented Jun 7, 2025

Closing this PR in favor of #1835

@comphead comphead closed this Jun 7, 2025
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.

Support map_values

3 participants