Skip to content

fix: support min/max for Float16 type - #12050

Merged
alamb merged 2 commits into
apache:mainfrom
korowa:float16-min-max
Aug 19, 2024
Merged

fix: support min/max for Float16 type#12050
alamb merged 2 commits into
apache:mainfrom
korowa:float16-min-max

Conversation

@korowa

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes#11764.

Rationale for this change

Float16 type support is missing in min/max groups and regular accumulators.

What changes are included in this PR?

Float16 type added to min/max accumulators.

minor: uncommented arrow_casts sqllogictests for float16

Are these changes tested?

Added sqllogictests

Are there any user-facing changes?

@github-actionsgithub-actionsBot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Aug 18, 2024

@alambalamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me -- thank you @korowa

arrow_cast(1, 'UInt64') as col_u64,
-- can't seem to cast to Float16 for some reason
-- arrow_cast(1.0, 'Float16') as col_f16,
arrow_cast(1.0, 'Float16') as col_f16,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎉

@alamb
alamb merged commit 7c5a8eb into apache:mainAug 19, 2024
@alamb

Copy link
Copy Markdown
Contributor

Thanks again @korowa

jayzhan211 added a commit to jayzhan211/datafusion that referenced this pull request Aug 20, 2024
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
jayzhan211 added a commit that referenced this pull request Aug 21, 2024
…nction, add `AggregateUDFImpl::is_null` (#11989)
* schema assertion and fix the mismatch from logical and physical
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add more msg
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* cleanup
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm test1
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* nullable for scalar func
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* nullable
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm field
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm unsafe block and use internal error
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm func_name
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm nullable option
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add test
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add more msg
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* fix test
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm row number
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* Update datafusion/expr/src/udaf.rs
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
* Update datafusion/expr/src/udaf.rs
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
* fix failed test from #12050
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* cleanup
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add doc
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
---------
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
wiedld pushed a commit to influxdata/arrow-datafusion that referenced this pull request Oct 4, 2024
…nction, add `AggregateUDFImpl::is_null` (apache#11989)
* schema assertion and fix the mismatch from logical and physical
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add more msg
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* cleanup
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm test1
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* nullable for scalar func
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* nullable
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm field
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm unsafe block and use internal error
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm func_name
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm nullable option
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add test
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add more msg
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* fix test
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* rm row number
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* Update datafusion/expr/src/udaf.rs
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
* Update datafusion/expr/src/udaf.rs
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
* fix failed test from apache#12050
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* cleanup
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
* add doc
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
---------
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functionsChanges to functions implementationsqllogictestSQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Min/Max accumulator not implemented for type Float16

2 participants

@korowa@alamb