FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
316 stars 136 forks source link

Add failing test case for issue #339 #340

Closed willsoto closed 2 years ago

willsoto commented 2 years ago

See #339 for more info

cowtowncoder commented 2 years ago

Hmmh. As per my comment on #339, I am NOT 100% sure this is a valid test. In particular, it will not add any sync or file header beyond initial header since the output is considered a single logical stream. But I guess I can just merge it and we can determine validity over time, at least there is a reproduction.

One thing I do need before merging (if not already done) is CLA, from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and the usual method is to print it, fill & sign, scan/photo, email to info at fasterxml dot com. Once I get that I can merge the test case. Thank you!

willsoto commented 2 years ago

I'm more than happy to tweak the test case as necessary.

I wasn't quite sure what your intention was with that comment - I thought you may have just been positing one way it could work.

cowtowncoder commented 2 years ago

@willsoto Right, my comment was bit ambiguous. My guess is that this has no chance of working as things are; but what I am not sure of is as to whether this should be the way to use API. To know that I'd need to spend some time again reading Avro spec wrt file encoding. Specifically about whether Avro has a "Stream" concept distinct from Arrays of things. If so, it would conceptually match Jackson's SequenceWriter. And from that, whether there is additional sync/marker requirement between elements -- it looks like there is, judging by exception message. That would require solving; Jackson does actually have the concept of "Root Value Separator"s which might work. Not exactly as-is, since separator is assumed to be String. But... maybe with some tweaks.

Since I don't quite know if this is valid or not, I'd be happy to include it under failing/ for storage, being easier to find whenever someone (myself or something else) has time to dig in and see if there's a solution.

willsoto commented 2 years ago

@cowtowncoder CLA sent.