Adds support for Ion 1.1 inline symbols to the system-level binary reader. #860
Conversation
| if (minor == 0) { | ||
| pendingIvmSids.add(SystemSymbols.ION_1_0_SID); | ||
| } else { | ||
| // TODO how should this be handled for Ion 1.1? | ||
| } |
There was a problem hiding this comment.
how should this be handled for Ion 1.1?
I have some ideas, but I haven't checked them for feasibility w.r.t. backwards compatibility with existing public APIs.
We don't actually need the SID of the IVM. We just need some value that represents which version it is. I don't think we should re-use IonEncodingVersion for this, but we could create an internal-only VersionMarker enum or something like that.
Alternately, maybe we could say that the system reader doesn't give you IVMs as symbols—it requires registering an IVM listener just like the regular reader.
There was a problem hiding this comment.
I think we're on the same page here. I would like if we could avoid requiring the Ion 1.1 writer to recognize SIDs as IVMs, while still allowing IVMs to be propagated from the system reader to a user-level writer. We may have to add some extra logic / callback registration between the writer and system reader to make that work, but it should be doable.
| * It is the caller's responsibility to ensure {@link #hasAnnotations()} returns | ||
| * true before calling this method. |
There was a problem hiding this comment.
Is it easier to implement this way, or can we just say that the consumer will receive no values if there are no annotations?
There was a problem hiding this comment.
This is an internal API, so I'm happy to have a stricter contract that cuts down on extra branching.
tgregg
force-pushed
the
ion-11-system-inline-symbols
branch
from
20461ea
to
c5d9d4a
Compare
Description of changes:
This requires checking with the "core" reader whether a symbol token was encoded as a SID or with inline text. Note: the system reader is not considered to be performance-critical.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.