Return scalar result when all inputs are constants in map and make_map
#11461
Conversation
|
cc @alamb @goldmedal |
Rachelint
changed the title
map and make_map udfsmap and make_map udfs
Rachelint
changed the title
map and make_map udfsmap and make_map
Rachelint
changed the title
map and make_mapmap and make_map
There was a problem hiding this comment.
Thanks, @Rachelint. LGTM. I guess this PR is also related to #11436. Maybe @dharanad could take a look at it.
| query error | ||
| SELECT map(column5, column6) FROM t; | ||
| # TODO: support array value | ||
| # ---- | ||
| # {k1:1, k2:2} | ||
| # {k3: 3} | ||
| # {k5: 5} |
There was a problem hiding this comment.
| query error | |
| SELECT map(column5, column6) FROM t; | |
| # TODO: support array value | |
| # ---- | |
| # {k1:1, k2:2} | |
| # {k3: 3} | |
| # {k5: 5} |
I think this test is the same as the above one. We can remove it.
|
Mind taking a quick look? @dharanad |
Taking a look |
datafusion/functions/src/core/map.rs
Outdated
| /// SELECT make_map('type', 'test') from test | ||
| /// ``` | ||
| /// We can evaluate the result of `make_map` directly. | ||
| #[inline] |
There was a problem hiding this comment.
Any specific reason for using inline over here ?
There was a problem hiding this comment.
Just a personal habit to mark the very short function, and try to reduce the function call cost(it will be really inlined or not... determinded by the compiler).
Will it cause some problems that I haven't noticed before?
There was a problem hiding this comment.
Since it more of a suggestion to complier, i dont think this should cause problems. TIL: Inlining small functions can provide small speed wins.
There was a problem hiding this comment.
I think there is rare case for inline. I perfer not to inline unless the benmark shows that compiler fail to do the good job for us
There was a problem hiding this comment.
Seem inline here is actually unnecessary according to the article mentioned by @jayzhan211 , removed.
|
Thank you @Rachelint @dharanad @goldmedal and @jayzhan211 |
…_map` (apache#11461) * return scalar result when all inputs are constants. * support convert map array to scalar. * disable the const evaluate for Map type before impl its hash calculation. * add tests in map.slt. * improve error return. * fix error. * fix remove unused import. * remove duplicated testcase. * remove inline.
…_map` (apache#11461) * return scalar result when all inputs are constants. * support convert map array to scalar. * disable the const evaluate for Map type before impl its hash calculation. * add tests in map.slt. * improve error return. * fix error. * fix remove unused import. * remove duplicated testcase. * remove inline.
…_map` (apache#11461) * return scalar result when all inputs are constants. * support convert map array to scalar. * disable the const evaluate for Map type before impl its hash calculation. * add tests in map.slt. * improve error return. * fix error. * fix remove unused import. * remove duplicated testcase. * remove inline.
…_map` (apache#11461) * return scalar result when all inputs are constants. * support convert map array to scalar. * disable the const evaluate for Map type before impl its hash calculation. * add tests in map.slt. * improve error return. * fix error. * fix remove unused import. * remove duplicated testcase. * remove inline.
Which issue does this PR close?
Closes #11460 #6485
Rationale for this change
See #11460
What changes are included in this PR?
See title.
Are these changes tested?
Test by new uts.
Are there any user-facing changes?
Can success to run sql like: