Copilot commented on code in PR #16210:
URL: https://github.com/apache/pinot/pull/16210#discussion_r2179573792
##########
pinot-core/src/main/java/org/apache/pinot/core/util/SegmentProcessorAvroUtils.java:
##########
@@ -83,45 +88,90 @@ public static Schema
convertPinotSchemaToAvroSchema(org.apache.pinot.spi.data.Sc
for (FieldSpec fieldSpec : orderedFieldSpecs) {
String name = fieldSpec.getName();
DataType storedType = fieldSpec.getDataType().getStoredType();
+
+ SchemaBuilder.BaseFieldTypeBuilder<Schema> type;
+ if (fieldSpec.isNullable()) {
+ type = fieldAssembler.name(name).type().nullable();
+ } else {
+ type = fieldAssembler.name(name).type();
+ }
+
+ String logicalType = "pinot." +
fieldSpec.getDataType().toString().toLowerCase(Locale.US);
if (fieldSpec.isSingleValueField()) {
switch (storedType) {
Review Comment:
BOOLEAN type is not handled in the switch-case for single-value fields,
leading to a RuntimeException when a BOOLEAN field is encountered. Add a case
for BOOLEAN (e.g., using .intBuilder() or .booleanBuilder()) and set the
logicalType property accordingly.
##########
pinot-core/src/test/java/org/apache/pinot/core/util/SegmentProcessorAvroUtilsTest.java:
##########
@@ -0,0 +1,220 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.util;
+
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
+import java.util.Locale;
+import org.apache.avro.generic.GenericData;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.testng.SkipException;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.*;
+
+
+public class SegmentProcessorAvroUtilsTest {
+
+
+ @DataProvider
+ public static Object[][] getTypeConversion() {
+ return new Object[][] {
+ new Object[] {FieldSpec.DataType.INT, org.apache.avro.Schema.Type.INT},
+ new Object[] {FieldSpec.DataType.LONG,
org.apache.avro.Schema.Type.LONG},
+ new Object[] {FieldSpec.DataType.FLOAT,
org.apache.avro.Schema.Type.FLOAT},
+ new Object[] {FieldSpec.DataType.DOUBLE,
org.apache.avro.Schema.Type.DOUBLE},
+ new Object[] {FieldSpec.DataType.STRING,
org.apache.avro.Schema.Type.STRING},
+ new Object[] {FieldSpec.DataType.BIG_DECIMAL,
org.apache.avro.Schema.Type.STRING},
+ new Object[] {FieldSpec.DataType.BYTES,
org.apache.avro.Schema.Type.BYTES},
+ new Object[] {FieldSpec.DataType.BOOLEAN,
org.apache.avro.Schema.Type.INT},
+ new Object[] {FieldSpec.DataType.JSON,
org.apache.avro.Schema.Type.STRING},
+ new Object[] {FieldSpec.DataType.TIMESTAMP,
org.apache.avro.Schema.Type.LONG}
+ };
+ }
+
+ @DataProvider
+ public static Object[][] getValueConversion() {
+ return new Object[][] {
+ new Object[] {FieldSpec.DataType.INT, 1, 1},
+ new Object[] {FieldSpec.DataType.LONG, 1L, 1L},
+ new Object[] {FieldSpec.DataType.FLOAT, 1.0f, 1.0f},
+ new Object[] {FieldSpec.DataType.DOUBLE, 1.0d, 1.0d},
+ new Object[] {FieldSpec.DataType.STRING, "test", "test"},
+ new Object[] {FieldSpec.DataType.BIG_DECIMAL, new BigDecimal("1.0"),
new BigDecimal("1.0")},
+ new Object[] {FieldSpec.DataType.BYTES, new byte[] {1, 2, 3},
ByteBuffer.wrap(new byte[] {1, 2, 3})},
+ new Object[] {FieldSpec.DataType.BOOLEAN, true, true},
+ new Object[] {FieldSpec.DataType.JSON, "{\"key\":\"value\"}",
"{\"key\":\"value\"}"},
+ new Object[] {FieldSpec.DataType.TIMESTAMP,
+ LocalDateTime.of(2025, 6, 25, 12, 0,
50).toInstant(ZoneOffset.UTC).toEpochMilli(),
+ LocalDateTime.of(2025, 6, 25, 12, 0,
50).toInstant(ZoneOffset.UTC).toEpochMilli()
+ }
+ };
+ }
+
+ @Test(dataProvider = "getValueConversion")
+ public void testConversion(
+ FieldSpec.DataType dataType,
+ Object initial,
+ Object expected
+ ) {
+ GenericRow genericRow = new GenericRow();
+ genericRow.putValue("testField", initial);
+
+ Schema.SchemaBuilder pinotSchemaBuilder = new Schema.SchemaBuilder();
+ pinotSchemaBuilder.setSchemaName("testSchema");
+ pinotSchemaBuilder.addDimensionField("testField", dataType, field ->
field.setNullable(true));
+
+ org.apache.avro.Schema avroSchema =
+
SegmentProcessorAvroUtils.convertPinotSchemaToAvroSchema(pinotSchemaBuilder.build());
+
+ GenericData.Record record =
SegmentProcessorAvroUtils.convertGenericRowToAvroRecord(
+ genericRow,
+ new GenericData.Record(avroSchema));
+
+ Object actualValue = record.get("testField");
+ assertEquals(actualValue, expected, "Unexpected value after conversion");
+ }
+
+ @Test(dataProvider = "getTypeConversion")
+ public void convertNullableSchema(
+ FieldSpec.DataType dataType,
+ org.apache.avro.Schema.Type expectedType
+ ) {
+ Schema.SchemaBuilder pinotSchemaBuilder = new Schema.SchemaBuilder();
+ pinotSchemaBuilder.setSchemaName("testSchema");
+ pinotSchemaBuilder.addDimensionField("testField", dataType, field ->
field.setNullable(true));
+
+ org.apache.avro.Schema avroSchema =
+
SegmentProcessorAvroUtils.convertPinotSchemaToAvroSchema(pinotSchemaBuilder.build());
+ assertNotNull(avroSchema);
+
+ String expectedLogicalType = "pinot." +
dataType.toString().toLowerCase(Locale.US);
+
+ org.apache.avro.Schema.Field avroField = avroSchema.getField("testField");
+ assertTrue(avroField.schema().isNullable(), "Field should be nullable");
+ assertEquals(avroField.schema().getTypes().size(), 2, "Field should have
two types (null and actual type)");
+
+ org.apache.avro.Schema firstFieldSchema =
avroField.schema().getTypes().get(0);
+ assertEquals(firstFieldSchema.getType(), expectedType, "Unexpected
physical type");
+ assertEquals(firstFieldSchema.getProp("logicalType"), expectedLogicalType,
+ "Unexpected logical type");
+ org.apache.avro.Schema secondFieldSchema =
avroField.schema().getTypes().get(1);
+ assertEquals(secondFieldSchema.getType(), org.apache.avro.Schema.Type.NULL,
+ "Second type should be null");
+ }
+
+ @Test(dataProvider = "getTypeConversion")
+ public void convertNonNullableSchema(
+ FieldSpec.DataType dataType,
+ org.apache.avro.Schema.Type exepctedType
Review Comment:
Typo in parameter name 'exepctedType'; should be 'expectedType' to improve
readability and avoid confusion.
```suggestion
org.apache.avro.Schema.Type expectedType
```
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
