Skip to content

Conversation

@mdepero
Copy link

@mdepero mdepero commented May 23, 2022

Uses nil for defLvls and repLvls when skipping boolean values, since the scratch buffer allocated for n boolean values when skipping is not large enough to hold n def and rep levels, resulting in an out of bounds panic when skipping too many rows.

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

Comment on lines 210 to 214
// buf is large enough to hold values, but not to hold def and rep lvls; use nil for them
vals, _, err := cr.ReadBatch(batch,
*(*[]bool)(unsafe.Pointer(&buf)),
arrow.Int16Traits.CastFromBytes(buf),
arrow.Int16Traits.CastFromBytes(buf))
nil,
nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the corresponding .tmpl file so that future calls to go generate don't clobber this. Otherwise this LGTM

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thank you!

@mdepero mdepero requested a review from zeroshade May 25, 2022 01:28
Copy link
Member

@zeroshade zeroshade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@zeroshade zeroshade closed this in 5994fd8 May 25, 2022
@zeroshade
Copy link
Member

Thanks @mdepero!

@ursabot
Copy link

ursabot commented May 25, 2022

Benchmark runs are scheduled for baseline = 7adda73 and contender = 5994fd8. 5994fd8 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.66% ⬆️1.91%] test-mac-arm
[Failed ⬇️5.15% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.12% ⬆️1.89%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 5994fd88 ec2-t3-xlarge-us-east-2
[Failed] 5994fd88 test-mac-arm
[Failed] 5994fd88 ursa-i9-9960x
[Finished] 5994fd88 ursa-thinkcentre-m75q
[Finished] 7adda73d ec2-t3-xlarge-us-east-2
[Failed] 7adda73d test-mac-arm
[Failed] 7adda73d ursa-i9-9960x
[Finished] 7adda73d ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants