Remove define_array_slice and reuse array_slice for array_pop_front/back
#8401
Conversation
jayzhan211
changed the title
array_elementdefine_array_sliceand reusearray_slice for array_pop_front/back
jayzhan211
changed the title
define_array_sliceand reusearray_slice for array_pop_front/backdefine_array_slice and reusearray_slice for array_pop_front/back
jayzhan211
changed the title
define_array_slice and reusearray_slice for array_pop_front/backdefine_array_slice and reuse array_slice for array_pop_front/back
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
jayzhan211
force-pushed
the
array-slice-refactor
branch
from
8c7e323
to
fc9b23f
Compare
There was a problem hiding this comment.
Thank you @jayzhan211 -- this looks great. I think we should add some more comments explaining what the functions are doing but given there are no existing comments I think this code is better than what is on main and therefore could be merged as is
The array code is really coming a long nicely
| let array = array.as_any().downcast_ref::<T>().unwrap(); | ||
|
|
||
| let array_type = array.data_type().clone(); | ||
| pub fn array_element(args: &[ArrayRef]) -> Result<ArrayRef> { |
There was a problem hiding this comment.
Does this need to be pub? If it does, could you please add a documentation comment explaining what the arguments are (namely that the first is a list and the second is an array of indexes?
There was a problem hiding this comment.
Yes, there is array_element sql function
| @@ -581,45 +502,118 @@ pub fn array_except(args: &[ArrayRef]) -> Result<ArrayRef> { | |||
|
|
|||
| pub fn array_slice(args: &[ArrayRef]) -> Result<ArrayRef> { | |||
There was a problem hiding this comment.
Also here I think it would help to add a comment explaining what the arguments are to this function and what the expected output is
Key details might be if the index values are 1 based or 0 based, for example
| false, | ||
| ) | ||
| let from_array = Int64Array::from(vec![1; list_array.len()]); | ||
| let to_array = Int64Array::from( |
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
| @@ -579,47 +507,136 @@ pub fn array_except(args: &[ArrayRef]) -> Result<ArrayRef> { | |||
| } | |||
| } | |||
|
|
|||
| /// array_slice SQL function | |||
…ont/back` (apache#8401) * array_element done Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * clippy Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * replace array_slice Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * fix get_indexed_field_empty_list Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * replace pop front and pop back Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * clippy Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * add doc and comment Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * fmt Signed-off-by: jayzhan211 <jayzhan211@gmail.com> --------- Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Which issue does this PR close?
Ref #7988
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?