Access a Map with Primitive type keys
#12259
Conversation
dharanad
changed the title
Map with a non-string keysMap with a non-string keys
dharanad
changed the title
Map with a non-string keysMap with a integer keys
dharanad
changed the title
Map with a integer keysMap with a integer keys
dharanad
changed the title
Map with a integer keysMap with Primitive keys
dharanad
changed the title
Map with Primitive keysMap with Primitive keys
dharanad
changed the title
Map with Primitive keysMap with Primitive type keys
|
@goldmedal Can you take a look |
|
@jayzhan211 Can you please me with a review here. |
| SELECT MAP { 1.0: 'a', 2.0: 'b', 3.0: 'c' }[2.0]; | ||
| ---- | ||
| b | ||
|
|
There was a problem hiding this comment.
what if SELECT MAP { 1.0: 'a', 2.0: 'b', 3.0: 'c' }[2];?
There was a problem hiding this comment.
Current implmentation will throw an error. Just checked DuckDB, it try to cast the index to keys data type.
| ---- | ||
| c | ||
|
|
||
| # FIXME: DuckDB handles this case |
There was a problem hiding this comment.
Oh i found out why.
Utf8 to numeric: strings that can’t be parsed to numbers return null, float strings in integer casts return null
Ref: https://docs.rs/arrow/latest/arrow/compute/kernels/cast/fn.cast_with_options.html
Taking a look into cast.rs for further reference
There was a problem hiding this comment.
It is well documented here under Behavious section 3rd Point, that float strings in integer casts return null
> select arrow_cast('1.0', 'Int64');
Arrow error: Cast error: Cannot cast string '1.0' to value of Int64 type
| let mut key_array: ArrayRef = name.to_array()?; | ||
| if key_array.data_type() != map_array.key_type() { | ||
| let pre_cast_dt = key_array.data_type().clone(); | ||
| if arrow::compute::kernels::cast::can_cast_types(key_array.data_type(), map_array.key_type()) { |
There was a problem hiding this comment.
Casting and type check should be handled in Signature not invoke.
| let Expr::Literal(name) = *index else { | ||
| return plan_err!("index should be a literal"); | ||
| }; | ||
| Ok(PlannerResult::Planned(get_field( |
There was a problem hiding this comment.
map_extract returns a list containing the value. Maybe can wrap it around get_field call and extract the value. But i am not sure, let me try it out.
|
I am trying to clean up the review queue, so marking this PR as draft as the CI tests are failing. Let me know if that wasn't right |
Which issue does this PR close?
Partially fixes #11785
Rationale for this change
What changes are included in this PR?
Added support to access map with primite type keys.
Using compound type as key is beyond the scope of this PR
Are these changes tested?
Uncommented existing test & ran
cargo testAre there any user-facing changes?