Repository navigation
feat(format): schema evolution for the Java row codec - #3714
stevenschlansker wants to merge 108 commits into
Conversation
7823b91 to
8be8335
Compare
5d7e5e3 to
4964041
Compare
|
@chaokunyang , this is not 100% done yet, but I think it is getting very close. Would you mind doing a design review to make sure that this feature will be acceptable to commit into the project? It is not a small feature, but I think the complexity is justified by giving the row format a low-cost method of achieving schema evolution. If there is some way to streamline the eventual code review of a large volume of LLM-typed code, please let me know. I intend to test this over the coming month or so internally, and mark the PR as ready once we have some real world experience running it. |
The spec said that every array and map body starts with an 8-byte prefix. "Array" and "map" also name the nested layouts, but Java writes the prefix only at the head of a top-level array or map payload. A runtime built from the old text would prefix a nested List<Bean> and fail to read Java payloads. Say that a nested array or map has no prefix.
AGENTS.md makes the exception type and the detection point of malformed input non-contracts. sizeBelowHashRejected pinned EncoderException and a reader index of exactly 4. Expect any RuntimeException, and only that the decode consumed the size prefix.
… a stream VersionedBinaryArrayEncoder and VersionedBinaryMapEncoder pointed the array or map at the body before they advanced the reader index. pointTo reads the element count or the key array size at once, so on a stream-backed buffer it read bytes that the stream had not loaded yet, and the decode returned wrong values. This is the same defect that the evolution-off array and map encoders have, copied into the new code. Advance the reader index first, which loads the body, then point at it, for the current layout and for an older layout.
…d maps out of decode BinaryRowEncoder keeps the projection and unknown-hash cases in decodeOtherSchema, so that decode holds only the current-schema path. Do the same in VersionedBinaryArrayEncoder and VersionedBinaryMapEncoder. No behavior change.
… comment The comment said that a JavaBean-style class names its members by the getter method. Descriptor takes the name of every class bean member from its field, so only an interface bean uses the accessor name.
… it requires lowerCamelCase The Java codec rejects only a member name that contains an underscore. Names such as fooURL and URLValue build, so "requires lowerCamelCase" in the spec, the guide, and the build error overstated the rule. State the real rule, and recommend lowerCamelCase names for portability.
"Peers need identical declared histories" contradicts the readers-first rollout, in which a reader knows more layouts than an older writer. Say that a reader decodes a payload when one of its layouts has the payload's hash.
…guide "Descriptor" is an internal term. Say that a history method matches the field name for a class or a record, and the accessor method name for an interface bean.
The schema evolution section of the guide showed the annotations but not the codec. Add an example that builds the codec with withSchemaEvolution(), encodes a value, and decodes a current and an older payload.
The comment said that the impl classes of two versions would collide on the simple name without the projection suffix. Each impl class is a member class of a uniquely named codec class, so they cannot collide. The suffix is there for readability.
No known-answer vector or test covered float16, float32, float64, duration, timestamp, or date32 in the strict hash. Add a test that schemas that differ only in one of these types, or only in its nullability, hash differently. Add one known-answer vector with all six types to the spec and the test, computed from the spec algorithm and confirmed in Java. No existing vector changes.
…n the compact sort
The spec said that in the compact format, two structs whose children
differ only in order match. The compact sort is stable, so children of
equal width keep the order of the codec: {x: int64, y: int64} and
{y: int64, x: int64} do not match, and the history discards the old
value instead of rejecting the type change. Correct the spec and the
validateNoTypeChange Javadoc, and add a test that pins this case in both
formats.
A projection reads every live field and every removed field that has a live field with the same name and declared type. It skips the slots of the other fields and does not read them. The comments on liveFieldNames() and in RowEncoderBuilder said that a projection reads and discards those fields, or that the skipped fields have no Java member. Also delete a stale sentence: a removed field sorts by the name of its history method, not by a name made from its wire name.
…tream The comment said that on a stream-backed buffer, the skip reads exactly the body from the stream. ForyInputStream.fillBuffer can read past the body when the stream returns more bytes. The skip moves the reader index past exactly the body, and loads the body from the stream if needed.
The prefix vector table did not say which row format its rows use. Say that they use the standard format, except for the row that names the compact format, as the strict hash vector table does.
The test asserted that the reader index is at least 4 after the failed decode. A decode that reads the hash and then moves the reader index back by the negative body size, from 12 to 8, passed. Decode from a stream that returns one byte per read, so the buffer holds only the bytes that the decode asked for, and assert that the reader index is at their end. The test still does not pin the exception type or an exact index.
No test decoded an evolution row from a stream-backed buffer: SchemaEvolutionTest.streamingOlderPayload reads a heap buffer. Add a row case to the stream decode test, which now covers rows, arrays, and maps, and rename it to streamedDecode. Rename streamingOlderPayload to sequentialOlderPayloads, which is what it tests. The row case fails if the decode reads the row before it moves the reader index past the body. It passes with either order of pointTo and the advance, because BinaryRow.pointTo only stores the offset.
classSuffix and nestedClassSuffixes were protected. Every class that reads or sets them is in org.apache.fory.format.encoder, so they do not need to be visible to subclasses in other packages.
The known-answer vector for date32, duration, float16, float32, float64, and timestamp pins the type ID of each type and the nullability input. The hash mixes nullability in one place for every type, so the separation test checks nothing that the vector and the other vectors do not already check.
An earlier commit changed the array and map codec builders to log the code generation time in microseconds, and renamed the duration variable in all three builders. The array and map logs on apache/main print milliseconds with a "us" label. That fix is not part of schema evolution, so restore the lines of apache/main and leave the fix to a separate change.
…sent field In a projection, an interface bean kept the default body of an accessor whose field the older payload lacks. The spec requires the absent value, as class beans already give.
The spec called the bytes after the size prefix the body and also the bytes after the hash. Say that the payload starts, after any size prefix, with the schema identifier, and that the hash precedes the body.
An array or map projection names a different bean than its codec, so bound the class name against the codec class name before its suffix. The reverse wire name mapping also capitalizes the letter after each underscore.
Map keys are version sites, the chain removal is until = 5 with four older layouts, the history collapses by strict hash, the decimal branch is in hashFieldShape, and the order-sensitivity test pins a documented property, not a reachable collision.
…he hash out of the body
806963d to
dc70e24
Compare
Opt in with
.withSchemaEvolution()on any row, array, or map codec builder. Live fields added after the first version carry@ForyRowVersion(since = N); removed fields are declared with@ForyRowVersion(until = N)on a class referenced from@ForyRowSchema(removedFields = ...). Older payloads are dispatched at read time. Standard and compact formats supported.With the flag off, the wire format and the generated codecs are unchanged. Two changes apply regardless of the flag:
TypeUtils.isSupportednow passes the type-resolution context to map keys and values, so interface beans work as map keys and values in every row codec, and fory-format compiles with<release>11</release>.Why?
Currently changing row format schema definition in any way invalidates all records
What does this PR do?
Propose a new concept of format versions, each succeeding version may add or remove fields from types, and deserialization machinery picks version based on schema hash
AI Contribution Checklist
AI Usage Disclosure
yes, I included a completed AI Contribution Checklist in this PR description and the requiredAI Usage Disclosure.yes, my PR description includes the requiredai_reviewsummary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes.Does this PR introduce any user-facing change?
New codec option: schema evolution. Some small annotations and a builder method.
Existing row format compatibility unchanged
The protocol (version model, wire names, field order, strict hash, and framing) is specified language-neutrally
in
docs/specification/row_format_spec.md. Java is the only implementation in this PR.Benchmark
withSchemaEvolution()is an opt-in feature that adds a new row-codec path; it does not modify any existing serialization hot path.There is no
apache/mainbaseline to compare against —SchemaEvolutionSuiteexerciseswithSchemaEvolution(), which does not existon
main— so the benchmark measures two things directly: the steady-state cost of enabling the flag, and projection-vs-currentparity.
Bounded JMH run (JDK 26, 2 forks × 4 iterations, 1s each,
-prof gc).B/opisgc.alloc.rate.norm(bytes allocated per operation).currentDecodecurrentDecodeNoEvolutionencodeencodeNoEvolutioncompactCurrentDecodecompactCurrentDecodeNoEvolutioncompactEncodecompactEncodeNoEvolutionolderDecodecompactOlderDecodeFindings:
B/opis byte-identical between the evolution-on and*NoEvolutionvariants on every path (decode 312/312, encode 152/152, compact decode 280/280, compact encode 144/144).
~10% confidence intervals on the no-evolution variants, so the throughput claim is "no measurable difference," not a tight bound;
allocation is exact.
vs 280 compact) because it reads the narrower V1 schema, not because projection is inherently cheaper. Each projection codec holds its
historical schema's precomputed row layout, so there is no per-decode rebuild.
Limitations
withSchemaEvolution()flag; the twoframings are not wire-compatible. A flag-mismatched peer fails loudly with
ClassNotCompatibleException(except evolution-off reading evolution-on bytes,which is undefined). Turning the flag on is a cutover, not a rolling change:
stop or drain producers, rewrite or stop reading stored payloads, then start
every peer with the flag on in a release that changes no schema.
schema change before any writer.
read evolution-enabled payloads until they implement the protocol in the row
format specification.
versions, so build time and memory grow as the product of the version counts of
the distinct nested versioned bean classes. Projection classes generate lazily,
on the first decode of each historical hash. To bound the growth, remove
Historyentries or remove or lower a live field's
since; the reader then rejects thepayloads with the retired layouts.
not yet agree on for acronyms, a leading capital, or non-ASCII capitals (xlang: the snake_case conversion of field names is not specified, and the runtimes do not agree #4187).
Ordinary lowerCamelCase names are portable.