ARROW-15528: [C++][R][Python] Update ExecPlan bindings #12694
Conversation
ahadnagy
changed the title
ahadnagy
changed the title
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
Can you expand on the comment by explaining why a user might want to attach metadata to the plan?
If we're using this for OT only right now we should say something like:
Optional metadata for the ExecPlan. This information will be attached to tracing output generated by the plan.
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
This is a slightly clear explanation of what is happening but it still doesn't explain why a user might want to do this.
I still don't think users will understand the purpose of the metadata. Why would a user want to attach metadata to a plan?
|
@westonpace I expanded the docstrings with more explanation, let me know if something's unclear in the descriptions. |
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
Thanks for sticking with this. I don't think I realized you were documenting internal methods before. I had thought this was user facing. The new description looks great though. One thought on the R implementation but otherwise this looks good.
| } | ||
|
|
||
| // [[arrow::export]] | ||
| std::shared_ptr<compute::ExecPlan> ExecPlan_create_with_metadata( |
There was a problem hiding this comment.
It seems rather odd to me that we would need two methods here but it may just be my ignorance of working with R. Wouldn't it be possible to send an empty value for metadata if the user doesn't want to specify it?
|
Closing because it has been untouched for a while, in case it's still relevant feel free to reopen and move it forward 👍 |
3 participants
This PR exposes the ability to attach optional metadata to ExecPlans. This metadata will be added to the OpenTelemetry tracing output, annotating the traces with information about the query and the environment (e.g TPC-H query id, scale factor, hardware details).