-
Notifications
You must be signed in to change notification settings - Fork 838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean the create_array
in IPC reader.
#2525
Clean the create_array
in IPC reader.
#2525
Conversation
Signed-off-by: remzi <13716567376yh@gmail.com>
Signed-off-by: remzi <13716567376yh@gmail.com>
Signed-off-by: remzi <13716567376yh@gmail.com>
ArrayData::builder(data_type.clone()) | ||
.len(length) | ||
.buffers(buffers[1..3].to_vec()) | ||
.offset(0) | ||
.null_bit_buffer((null_count > 0).then(|| buffers[0].clone())) | ||
.null_bit_buffer(null_buffer) | ||
.build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we only validate the data when building these 4 types array, but for other types, we use build_uncheked
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should probably be validating all of them, I'm not sure why we aren't validating some of them... IPC is inherently not trusted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will file an issue to track it.
Benchmark runs are scheduled for baseline = 98de2e3 and contender = 7cfe3cf. 7cfe3cf is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
None.
What changes are included in this PR?
Did some code cleaning work when I learned the IPC reader code.
Are there any user-facing changes?
No.