tarun11Mavani commented on code in PR #18710: URL: https://github.com/apache/pinot/pull/18710#discussion_r3386302499
########## pinot-spi/src/main/java/org/apache/pinot/spi/data/OpenStructTypeInference.java: ########## @@ -0,0 +1,68 @@ +/** + * 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.spi.data; + +import javax.annotation.Nullable; +import org.apache.pinot.spi.utils.PinotDataType; + + +/// Infers the {@link FieldSpec.DataType} for an OPEN_STRUCT key from raw ingested values when the key has +/// no declared child {@link FieldSpec}. This is OPEN_STRUCT-specific policy (it keeps TIMESTAMP, folds +/// DATE/TIME/UUID to STRING, widens BYTE/CHARACTER/SHORT to INT, and returns {@code null} for values that +/// cannot be represented as a stored column type), distinct from the JSON-node-based inference in +/// {@code JsonUtils.valueOf}. +public final class OpenStructTypeInference { + private OpenStructTypeInference() { + } + + /// Infers the {@link FieldSpec.DataType} from a raw ingested value. Returns {@code null} when the value + /// cannot be represented as a stored column type; callers decide whether to drop the entry or fall back + /// to a default (e.g. STRING). + @Nullable + public static FieldSpec.DataType inferDataType(Object rawValue) { + switch (PinotDataType.getSingleValueType(rawValue)) { + case INTEGER: + case BYTE: + case CHARACTER: + case SHORT: + return FieldSpec.DataType.INT; + case LONG: + return FieldSpec.DataType.LONG; + case FLOAT: + return FieldSpec.DataType.FLOAT; + case DOUBLE: + return FieldSpec.DataType.DOUBLE; + case BIG_DECIMAL: + return FieldSpec.DataType.BIG_DECIMAL; + case BOOLEAN: + return FieldSpec.DataType.BOOLEAN; + case TIMESTAMP: + return FieldSpec.DataType.TIMESTAMP; + case STRING: + case DATE: + case TIME: Review Comment: No user-facing config — this is caller-side behavior in `MutableOpenStructIndex.index()` and `OpenStructColumnSplitter` (PR 2b). When inference returns null, they skip the key-value pair for that document. The Javadoc describes the API contract (null = unrepresentable), not a configurable policy. One thing I'd like everyones thoughts on — standard Pinot columns throw `UnsupportedOperationException` from `PinotDataType.convert()` for incompatible conversions, and `MutableSegmentImpl` respects `continueOnError` to decide whether to fail the row. Currently OPEN_STRUCT always silently skips, regardless of `continueOnError`. I'm thinking about a middle ground: - **Inference failures** (type genuinely unrepresentable — e.g., nested Map, arbitrary Object): always skip. This is expected with schema-less data, and failing ingestion here would make OPEN_STRUCT unusable for heterogeneous maps. - **Coercion failures** (type was resolved but value conversion failed — e.g., declared child type is INT but value is `"not_a_number"`): respect `continueOnError`. This is a data quality issue, same as regular columns. Would appreciate your perspective on whether this distinction makes sense. If so, I'll thread `continueOnError` into `MutableOpenStructIndex` for coercion failures in PR 2b. -- 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]
