Skip to content

Conversation

@ovr
Copy link
Contributor

@ovr ovr commented Jan 10, 2021

The function called create_batch_empty is used inside hash_aggregate to make an empty batch from the provided schema which contains type for columns. In this PR I added support for:

  • DataType::Decimal
  • DataType::Timestamp with all TimeUnits with passing timezone
  • DataType::Date32
  • DataType::Date64
  • DataType::Time32
  • DataType::Time64

In apache#9114, I've prepared support for List/LargeList/FixedSizeList, but will be great to support more types
@github-actions
Copy link

@codecov-io
Copy link

codecov-io commented Jan 10, 2021

Codecov Report

Merging #9156 (67e995b) into master (9560204) will increase coverage by 0.00%.
The diff coverage is 73.58%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9156   +/-   ##
=======================================
  Coverage   81.80%   81.81%           
=======================================
  Files         214      214           
  Lines       51383    51436   +53     
=======================================
+ Hits        42034    42081   +47     
- Misses       9349     9355    +6     
Impacted Files Coverage Δ
rust/datafusion/src/physical_plan/common.rs 78.78% <73.58%> (+13.57%) ⬆️
rust/parquet/src/encodings/encoding.rs 95.24% <0.00%> (-0.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98f01c5...67e995b. Read the comment docs.

@andygrove
Copy link
Member

@ovr Please update the PR description - if you look at other open PRs you can get a better idea for how these are normally written. These descriptions are important because they become the commit message when the PR is merged. Thanks!

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks good to me -- thanks @ovr -- are you preparing to implement group by for these time datatypes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants