Generate Avro schemas from Pinot logical data types (#19071)#19073
Open
xiangfu0 wants to merge 1 commit into
Open
Generate Avro schemas from Pinot logical data types (#19071)#19073xiangfu0 wants to merge 1 commit into
xiangfu0 wants to merge 1 commit into
Conversation
AvroUtils.getAvroSchemaFromPinotSchema and
SegmentProcessorAvroUtils.convertPinotSchemaToAvroSchema switched on
fieldSpec.getDataType().getStoredType(), so the generated Avro schema
described Pinot's physical storage rather than its logical type: BOOLEAN
was emitted as Avro int, TIMESTAMP as a bare long, UUID needed a one-off
special case, and BIG_DECIMAL fell through as unsupported.
Drive schema generation on the original (logical) DataType via a single
shared mapping, AvroSchemaUtil.toAvroSchema(DataType/FieldSpec):
BOOLEAN -> boolean
TIMESTAMP -> long{logicalType:timestamp-millis}
BIG_DECIMAL -> bytes{logicalType:big-decimal}
UUID -> string{logicalType:uuid}
with the rest unchanged. AvroUtils and the AvroSchemaUtil JSON view now
delegate to it, dropping the one-off UUID branches.
Coordinate the writer/value side to match: register Avro's
BigDecimalConversion alongside the existing UuidConversion on the shared
data model (getAvroDataModel), and make convertGenericRowToAvroRecord
schema-driven -- coerce Pinot's stored int 0/1 to Boolean for BOOLEAN
fields and wrap byte[] as ByteBuffer for BYTES, while leaving values a
registered Conversion owns (UUID byte[], BigDecimal) untouched. MV values
keep a zero-copy view unless their element type actually needs a
transform.
SegmentProcessorAvroUtils.convertPinotSchemaToAvroSchema keeps a private
copy of the mapping because pinot-core cannot depend on the pinot-avro-base
plugin (and vice versa); a cross-module test pins the two together.
The recommender's AvroWriter is intentionally left as-is: its data
generator cannot produce BIG_DECIMAL values, so registering the
conversion there would be untested/unreachable code.
Adds Pinot-schema-to-Avro-schema assertions for every logical type
(SV and MV) and end-to-end write/read round-trips through the shared data
model and the production AvroRecordReader / segment build path.
Note: the segment->Avro and segment->Parquet export tools now emit
boolean (was int) for BOOLEAN and timestamp-millis (was bare long) for
TIMESTAMP; both remain self-describing and decodable.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19073 +/- ##
============================================
+ Coverage 65.45% 65.52% +0.06%
Complexity 1421 1421
============================================
Files 3425 3425
Lines 216303 216252 -51
Branches 34266 34261 -5
============================================
+ Hits 141590 141689 +99
+ Misses 63317 63175 -142
+ Partials 11396 11388 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Jackie-Jiang
added
enhancement
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Resolves #19071.
AvroUtils.getAvroSchemaFromPinotSchemaandSegmentProcessorAvroUtils.convertPinotSchemaToAvroSchemaswitched onfieldSpec.getDataType().getStoredType(), so the generated Avro schema described Pinot's physical storage rather than its logical type:BOOLEANwas emitted as AvrointTIMESTAMPwas emitted as a barelong(no logical type)UUIDneeded a one-off special caseBIG_DECIMALfell through as unsupportedChange
Drive schema generation on the original (logical)
DataTypevia a single shared mapping,AvroSchemaUtil.toAvroSchema(DataType/FieldSpec):booleanBoolean(coerced from storedint0/1)long{logicalType:timestamp-millis}Longepoch millisbytes{logicalType:big-decimal}BigDecimalviaBigDecimalConversionstring{logicalType:uuid}byte[]viaUuidConversionINT/LONG/FLOAT/DOUBLE/STRING/JSON/BYTES are unchanged.
AvroUtils.getAvroSchemaFromPinotSchemaand theAvroSchemaUtil.toAvroSchemaJsonObjectJSON view now delegate to this one mapping, dropping the previous one-off UUID branches.Coordinate the writer/value side to match:
BigDecimalConversionalongside the existingUuidConversionon the shared data model (SegmentProcessorAvroUtils.getAvroDataModel).convertGenericRowToAvroRecordschema-driven: coerce Pinot's storedint0/1 toBooleanfor BOOLEAN fields and wrapbyte[]asByteBufferfor BYTES, while leaving values a registeredConversionowns (UUIDbyte[],BigDecimal) untouched. MV values keep a zero-copy view unless their element type actually needs a transform.SegmentProcessorAvroUtils.convertPinotSchemaToAvroSchemakeeps a private copy of the mapping becausepinot-corecannot depend on thepinot-avro-baseplugin (and vice-versa); a cross-module test pins the two together so they can't drift.Compatibility
AvroSchemaUtil.valueOf(Schema)) is deliberately left one-way, preserving existing Pinot schema inference from Avro data.segment -> Avro/segment -> Parquetexport tools now emitboolean(wasint) for BOOLEAN andtimestamp-millis(was a barelong) for TIMESTAMP. Both stay self-describing and decodable.AvroWriteris intentionally not touched: its data generator cannot produceBIG_DECIMALvalues, so wiring the conversion there would be untested/unreachable. The anonymizer's separategetAvroSchemaFromPinotSchemais likewise out of scope — it has its own value-generation model and fails loudly rather than silently mis-mapping.Tests
AvroSchemaUtilTest,AvroUtilsTest,SegmentProcessorAvroUtilsTest).SegmentProcessorAvroUtils's mapping toAvroUtils's.AvroRecordReader, exercising the same conversions as the segment-processing writers (SegmentProcessorAvroUtilsTest), plus a full segment-build round-trip (FileBasedSegmentWriterTest) and the Avro + Parquet converters incl.AvroParquetWriter.withDataModel(PinotSegmentConverterTest). BigDecimal scale preservation and MVbyte[]element-wise equality are asserted.