You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/10/19 18:44:27 UTC

[GitHub] [arrow] jorgecarleitao commented on pull request #8430: ARROW-10249: [Rust] Support nested dictionaries inside list arrays

jorgecarleitao commented on pull request #8430:
URL: https://github.com/apache/arrow/pull/8430#issuecomment-712369692


   @vertexclique , I think that the feature is appreciated, but the implementation shows some underlying issue with the API to build arrays.
   
   IMO, In light of this PR we should revisit the API before merging this to ensure that we can create nested structures without `unsafe`, and, if we need an `unsafe`, that we use it in a well-defined, limited scope.
   
   If we accept this, IMO we are opening the door to all kind of usage of `unsafe` in high-level APIs, which is not only an anti-pattern in Rust, but also discouraged except in well defined use-cases, as it risks UB. Note that I am not criticizing this implementation in particular, but that if anyone changes this, it requires a significant care by everyone involved to ensure no UB.
   
   In other words, this implementation adds technical debt that I (and I alone) will be unable to pay back, which means that I cannot commit at maintaining this change. I would obviously not block others from approving it and merging if they feel that they can maintain this change.
   
   cc @paddyhoran
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org