Conversation
changhiskhan
left a comment
changhiskhan
left a comment
There was a problem hiding this comment.
a couple of questions
| } | ||
|
|
||
| ARROW_ASSIGN_OR_RAISE(auto datum, | ||
| ::arrow::compute::ExecuteScalarExpression(expression, *schema(), batch)); |
There was a problem hiding this comment.
does this bind the expression or is it required to be bound before the AddColumn method is called?
There was a problem hiding this comment.
the expression is bound when entering this AddColumn method.
| ARROW_ASSIGN_OR_RAISE(arr, CreateArray(datum.scalar(), batch->num_rows())); | ||
| } else if (datum.is_chunked_array()) { | ||
| auto chunked_arr = datum.chunked_array(); | ||
| ARROW_ASSIGN_OR_RAISE(arr, ::arrow::Concatenate(chunked_arr->chunks())); |
There was a problem hiding this comment.
ExtensionArray's cannot be concatenated currently - tho compute expressions won't either so ExtensionArray's probably won't make it past ExecuteScalarExpression?
There was a problem hiding this comment.
Let's add a test as follow up? But also, there is no function / kernel is available for extension types right now, this method might fail earlier.
| std::move(new_field)); | ||
| } | ||
|
|
||
| ::arrow::Result<std::shared_ptr<LanceDataset>> LanceDataset::AddColumn( |
There was a problem hiding this comment.
again the problem here is if the compute expression contains aggregates
There was a problem hiding this comment.
This can be checked via bool Expression::IsScalarExpression() const.
I can throw a invalid status from AddColumn
| ARROW_ASSIGN_OR_RAISE(auto datum, | ||
| ::arrow::compute::ExecuteScalarExpression(expression, *schema(), batch)); | ||
| std::shared_ptr<::arrow::Array> arr; | ||
| if (datum.is_scalar()) { |
There was a problem hiding this comment.
ok so this is a constant literal value?
There was a problem hiding this comment.
Yes, this is for case like AddColumn(field, pc::literal(1234)).
changhiskhan
left a comment
changhiskhan
left a comment
There was a problem hiding this comment.
should be safe if you just add the check/raise on column aggregates
added |
2 participants
Closes #319 and #321