[GitHub] [arrow] Alfred-Mountfield commented on pull request #12451: ARROW-15705: [JavaScript] Allowing appending null on children in a StructBuilder

[GitBox]

Alfred-Mountfield commented on pull request #12451:
URL: https://github.com/apache/arrow/pull/12451#issuecomment-1060572661


   Apologies for the late response. Thank you for pushing for more concrete 
proof that this PR achieves its intention.
   
   While trying to create a better test-case for you I realise we have a large 
quantity of errors within our uses of the library that have only been exposed 
to me while trying to create a test-case within the arrow project itself in an 
IDE with proper intellisense. 
   
   I think the code examples I have given (i.e. the StackBlitz links) aren't 
valid due to errors such as these:
   <img width="1119" alt="image" 
src="https://user-images.githubusercontent.com/25749103/157021673-7e14f2ae-ca97-4321-8fae-2bf64dcf8745.png";>
   <img width="537" alt="image" 
src="https://user-images.githubusercontent.com/25749103/157021284-5a0c8df4-c424-437a-87ff-944d80a22975.png";>
   <img width="699" alt="image" 
src="https://user-images.githubusercontent.com/25749103/157021782-25003004-84fc-4a97-b0a6-711ff7fbf0c9.png";>
   
   Unfortunately, using the JS Arrow packages don't really give any indication 
that doing things like that are incorrect. Not trying to restate the obvious 
benefits of using TypeScript, but I do worry a bit that the arrow package lends 
itself to so many errors that fail somewhat silently in plain JS because of 
things like abstract classes. I think having better documentation showing more 
use-cases and trying to convey convention would help a lot with this. Is there 
somewhere I can raise this properly (as this PR doesn't feel like the right 
place to start that discussion), should I just open a Jira ticket or will that 
likely languish?
   
   Going back to this PR, I'm going to investigate the methods a bit more 
deeply, and try and understand the library's implementation a bit better when 
it comes to things like `setValid` etc. I worry that the spec is a little too 
vague when it comes to the specifics of the lengths of buffers in things like 
`fixedSizeList`s and `Struct`s with null elements, and it's possible the 
different language impls have conflicting interpretations. I'll do my 
investigation in TS (rather than JS as I was before) and post again afterwards 
:)


-- 
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.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

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