PIP-177: Add the classLoader field for SchemaDefinition

[coderzc]

Mailing list thread: https://lists.apache.org/thread/ydslz3h8ymjjwm5ng02kwszmc5j5hy30

Motivation

Now, don‘t register logical type conversions when use SchemaDefinition.<T>builder().withJsonDef() to create the schema, beacase it without classLoader param. (e.g: #15899)

See:

pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/reader/AvroReader.java

Lines 58 to 68 in 04aa9e8

	public AvroReader(Schema writerSchema, Schema readerSchema, ClassLoader classLoader,
	boolean jsr310ConversionEnabled) {
	this.schema = readerSchema;
	if (classLoader != null) {
	ReflectData reflectData = new ReflectData(classLoader);
	AvroSchema.addLogicalTypeConversions(reflectData, jsr310ConversionEnabled);
	this.reader = new ReflectDatumReader<>(writerSchema, readerSchema, reflectData);
	} else {
	this.reader = new ReflectDatumReader<>(writerSchema, readerSchema);
	}
	}

We can add the classLoader field for SchemaDefinition, user can manually pass a classLoader to register logical type conversions

Goal

This proposes to add the classLoader field for SchemaDefinition. When using SchemaDefinition.<T>builder().withJsonDef() to create the schema it must manually specify a classLoader otherwise, the converter will not work.

The priority of the classLoader field will be higher than by the pojoClass.getClassLoader().

API Changes

public class SchemaDefinitionBuilder {
    //....

    /**
     * Set schema of pojo classLoader.
     *
     * @param classLoader pojo classLoader
     *
     * @return schema definition builder
     */
    SchemaDefinitionBuilder<T> withClassLoader(ClassLoader classLoader);
}

public class SchemaDefinition {
    //....

    /**
     * Get pojo classLoader.
     *
     * @return pojo schema
     */
    ClassLoader getClassLoader();
}

Implementation

Add the classloader field for SchemaDefinition.

[eolivelli]

how does this work with other Schema types ?

[coderzc]

how does this work with other Schema types ?

@eolivelli it only work with the avro schema

[coderzc]

@eolivelli Do you have any questions about this proposal?

[eolivelli]

LGTM thanks

[coderzc]

@codelipenghui @congbobo184 @eolivelli @mattisonchao @shibd I sent a PIP vote mail, take a look at it.
mail: https://lists.apache.org/thread/ydslz3h8ymjjwm5ng02kwszmc5j5hy30

[congbobo184]

+1 (none-binding) Thanks, Bo

…

2022年6月14日 下午9:54，Cong Zhao ***@***.***> 写道： Motivation Now, don‘t register logical type conversions when use SchemaDefinition.<T>builder().withJsonDef() to create the schema, beacase it without classLoader param. See: https://github.com/apache/pulsar/blob/04aa9e8e51869d1621a7e25402a656084eebfc09/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/reader/AvroReader.java#L58-L68 <https://github.com/apache/pulsar/blob/04aa9e8e51869d1621a7e25402a656084eebfc09/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/reader/AvroReader.java#L58-L68> We can add the classLoader field for SchemaDefinition, user can manually pass a classLoader to register logical type conversions Goal This proposes to add the classLoader field for SchemaDefinition. When using SchemaDefinition.<T>builder().withJsonDef() to create the schema it must manually specify a classLoader otherwise, the converter will not work. The priority of the classLoader field will be higher than by the pojoClass.getClassLoader(). API Changes public class SchemaDefinition { //.... /** * Set schema of pojo classLoader. * * @param classLoader pojo classLoader * * @return schema definition builder */ SchemaDefinitionBuilder<T> withClassLoader(ClassLoader classLoader); } public class SchemaDefinition { //.... /** * Get pojo classLoader. * * @return pojo schema */ ClassLoader getClassLoader(); } Implementation Add the classloader field for SchemaDefinition. — Reply to this email directly, view it on GitHub <#16058>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJKEXQV6UUX2KA4HKYBUUDLVPCFJFANCNFSM5YXZWOKA>. You are receiving this because you are subscribed to this thread.