danny0405 commented on code in PR #19144:
URL: https://github.com/apache/hudi/pull/19144#discussion_r3520508397
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaCache.java:
##########
@@ -28,21 +29,77 @@
* <p>This is a global cache which works for a JVM lifecycle.
* A collection of schema instances are maintained.
*
+ * <p>This value-keyed pool is the canonicalization mechanism behind
+ * {@link HoodieSchema#fromAvroSchema}, and can also be used directly to
intern schemas
+ * produced without an Avro source (builders, converters).
+ *
+ * <p>Interning is never lossy: entries are keyed on the schema's full
serialized content,
Review Comment:
the intern should be used on hotspot code path that schema is mostly used in
read/write path, so the missing of comment or doc should be fine, let's not
introduce unnecessary complexities.
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchema.java:
##########
@@ -1644,7 +1689,10 @@ public HoodieSchema parse(String jsonSchema) {
try {
Schema avroSchema = avroParser.parse(jsonSchema);
- return fromAvroSchema(avroSchema);
+ // Return a fresh, owned instance rather than the interned canonical
one: parsed schemas
+ // are frequently mutated by callers (e.g. addProp in client-init
callbacks), and mutating
+ // a shared interned instance would corrupt it for every other holder
of the same schema.
+ return buildFromAvroSchema(avroSchema);
Review Comment:
> parsed schemas
> // are frequently mutated by callers (e.g. addProp in client-init
callbacks), and mutating
> // a shared interned insta
shouldn't the schema always been immutable once it is built?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
