-
Notifications
You must be signed in to change notification settings - Fork 67
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
Use demand in vector.read #4850
Conversation
d7aa200
to
451f865
Compare
@brimdata/zed This is ready for review, but before merging I'll need to rebase it on #4833 to make the vng tests use the new fuzz package. |
6c2f836
to
e1f249d
Compare
… without circular dependencies
72d0e38
to
93f3b0d
Compare
@nwt I finished rebasing this on the other two PRs. |
Co-authored-by: Noah Treuhaft <[email protected]>
Co-authored-by: Noah Treuhaft <[email protected]>
Co-authored-by: Noah Treuhaft <[email protected]>
Co-authored-by: Noah Treuhaft <[email protected]>
Co-authored-by: Noah Treuhaft <[email protected]>
Co-authored-by: Noah Treuhaft <[email protected]>
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.
I'd like to have the NewReader(zctx *zed.Context, r io.Reader, d demand.Demand)
change but then .
return a.Values(), nil | ||
} | ||
|
||
func WriteZNG(t *testing.T, valuesIn []zed.Value, buf *bytes.Buffer) { |
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.
Nits:
- Could use io.Writer here to make this a little more flexible.
- We usually put the destination parameter first (as with zio.Copy).
- We usually use val as the name for zed.Value slice.
func WriteZNG(t *testing.T, valuesIn []zed.Value, buf *bytes.Buffer) { | |
func WriteZNG(t *testing.T, w io.Writer, vals []zed.Value) { |
9566145
to
2c177b1
Compare
Hooking up demand to vector.Read.
This doesn't yet actually do anything for
zq
orzed query
. I can work on the latter, and @nwt is thinking about how to refactor to makezq
possible.