Add partial-match traversal of large bytes #375
Conversation
willscott
changed the title
| } | ||
|
|
||
| // ReadAt provides the io.ReadAt method over a ReadSeeker. | ||
| // This implementation does not support concurrent calls to `ReadAt`, |
There was a problem hiding this comment.
So my one worry here is: what are the odds the consumer of this code reads this comment, and not the comment on golang's ReaderAt interface -- is there a chance for misuse through the chain of readerAt -> SectionReader -> NewBytesFromReader? SectionReader certainly seems to use ReadAt to implement seek+read for example.
A seperate question: should AsLargeBytes return a ReadSeeker that is also a ReaderAt? os.File satisfies both. new section reader satisfies both.
Non blocking, but we should look at figuring this out when we can.
There was a problem hiding this comment.
@mvdan argued that ReadSeeker is a more common and lower effort to ask implementations of node to implement. This wrapper is internal (since the struct type is not exported) and so is only for use by the Slice matcher to wrap a SectionReader around a node's ReadSeeker. As such, I believe there's minimal potential for misuse.
| @@ -106,6 +106,12 @@ type Selector interface { | |||
| // Only "Matcher" clauses actually implement this in a way that ever returns "true". | |||
| // See the Selector specs for discussion on "matched" vs "reached"/"visited" nodes. | |||
| Decide(node datamodel.Node) bool | |||
There was a problem hiding this comment.
Are we using decide any more? Should we just delete it? No one uses compile selectors outside of go-ipld-prime I think
There was a problem hiding this comment.
i don't see any uses of Decide in the go-ipld-prime directly before this PR either. I think the thought is to transition to Match as the signature that we actually need for these walks.
My preference would be to mark this as deprecated rather than turn this into a backwards-incompatible change.
hannahhoward
force-pushed
the
feat/largeByteTraversal
branch
from
1a03108
to
c3d07b4
Compare
|
Also I rebased this for you since I was the one who created your merge conflict :) |
…me into feat/largeByteTraversal
|
Crosslinking (since I just had to go re-find it): spec happened in: ipld/ipld#184 |
streamBytesparallel toplainBytesfor byte nodes backed byio.ReadSeekersrather than[]byteSlicefor subset matchesDecide(node) boolmethod for matching toMatch(node) (node, error), which allows a match to be done on only a subset of a node.walk_advto make use of theSliceselector by way of theMatchmethod.