xiangfu0 commented on code in PR #18436: URL: https://github.com/apache/pinot/pull/18436#discussion_r3201306926
########## pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/BaseRecordExtractor.java: ########## @@ -18,46 +18,16 @@ */ package org.apache.pinot.spi.data.readers; -import com.google.common.collect.Maps; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.nio.ByteBuffer; import java.sql.Timestamp; import java.util.Base64; -import java.util.Collection; -import java.util.Map; import java.util.Set; import javax.annotation.Nullable; import org.apache.commons.collections4.CollectionUtils; -import org.apache.commons.lang3.ArrayUtils; -/// Default [RecordExtractor] implementation. Subclasses override only the bits the format needs. -/// -/// [#convert] dispatches by shape: -/// - multi-value (`Collection` / non-`byte[]` array) → [#convertMultiValue] → `Object[]` (each element recursed) -/// - map → [#convertMap] → `Map<String, Object>` (keys converted via [#convertSingleValue] then stringified via -/// [#stringifyMapKey], each value recursed) -/// - nested record → [#convertRecord] (throws by default; override for formats with nested records) -/// - everything else → [#convertSingleValue] -/// -/// [#convertSingleValue] applies universal normalizations for primitive types that any format extractor might -/// produce: -/// - `Byte` / `Short` widen to `Integer` so all small ints unify behind a single Pinot type -/// - `BigInteger` widens to `BigDecimal` (Pinot has no `BigInteger` data type; downstream transforms handle -/// `BigDecimal` natively) -/// - other `Number` (`Integer` / `Long` / `Float` / `Double` / `BigDecimal`) passes through -/// - `Boolean` passes through -/// - `byte[]` passes through -/// - `ByteBuffer` materializes to `byte[]` (sliced so the source buffer's position is not advanced) -/// - everything else falls back to `value.toString()` -/// -/// **Logical types (DECIMAL / TIMESTAMP / DATE / TIME / UUID) are NOT handled here** — see [RecordExtractor] -/// for the contract. Format-specific extractors do the native-to-contract conversion themselves (e.g. the -/// Avro extractor walks the schema in its own `extract` and never reaches this dispatcher). +/// Default [RecordExtractor] base providing include-list resolution via [#init] and the [#stringifyMapKey] helper. Review Comment: This trims `BaseRecordExtractor` in `pinot-spi` down to `init()` + `stringifyMapKey()`, which removes the protected `convert*` helpers that existing external extractors can override or call today. Any plugin extending `BaseRecordExtractor` will stop compiling or loading on upgrade even if its behavior did not change. Please preserve the old protected methods as deprecated bridges until downstream record-extractor implementations have a compatibility window. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
