github-actions[bot] commented on code in PR #70185: URL: https://github.com/apache/doris/pull/70185#discussion_r4466267787
########## contrib/apache-orc: ########## @@ -1 +1 @@ -Subproject commit be0f1b73a7aeb78824a03e0dcb692c50a176d513 +Subproject commit 5962bb071d6345cc29c5353d64d85a203525e4d9 Review Comment: [P1] Add the ARM libdeflate link with this ORC pin This pin also imports [34a46c9](https://github.com/apache/doris-thirdparty/commit/34a46c9e2de3214eea527f50c45c94264c14cdcd), which removes the architecture guards around the libdeflate decoder and its ZLIB factory. However, `be/cmake/thirdparty.cmake:192-195` still adds `deflate` only for x86/x86_64, and the pinned ORC target does not declare a libdeflate dependency. AArch64/arm64 BE links therefore gain unresolved `libdeflate_alloc_decompressor`, `libdeflate_free_decompressor` and `libdeflate_deflate_decompress` references. Include libdeflate in the ARM link as part of this update, or retain the architecture fallback. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/es/EsMappingUtils.java: ########## @@ -0,0 +1,442 @@ +// 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.doris.datasource.es; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Set; + +/** + * ES mapping parsing and field resolution utilities. + */ +public final class EsMappingUtils { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + public static final Set<String> DEFAULT_DOCVALUE_DISABLED_FIELDS = + new HashSet<>(Arrays.asList("text", "annotated_text", "match_only_text")); + + private static final Set<String> ROOT_MAPPING_OBJECT_PARAMETERS = new HashSet<>(Arrays.asList( + "properties", "runtime", "_source", "_routing", "_field_names", "_size", "_data_stream_timestamp")); + + private static final Set<String> DOCVALUE_QUERY_TYPES = new HashSet<>(Arrays.asList( + "keyword", "byte", "short", "integer", "long", "unsigned_long", "float", "double", + "half_float", "scaled_float", "date", "date_nanos", "boolean", "ip", "geo_point")); + + private static final Set<String> IGNORE_MALFORMED_TYPES = new HashSet<>(Arrays.asList( + "byte", "short", "integer", "long", "unsigned_long", "float", "double", "half_float", "scaled_float", + "boolean", "date", "date_nanos", "ip", "geo_point", "geo_shape")); + + private EsMappingUtils() { + } + + /** + * Parse the top-level "mappings" node from an index mapping JSON response. + */ + public static ObjectNode getMapping(String indexMapping) { + try { + ObjectNode jsonNodes = (ObjectNode) MAPPER.readTree(indexMapping); + // Alias mapping response order is unspecified. A bare/typed empty backing can appear + // first; select a populated schema without discarding its _meta array annotations. + for (JsonNode index : jsonNodes) { + ObjectNode mapping = (ObjectNode) index.get("mappings"); + ObjectNode root = getRootSchema(mapping.deepCopy(), null, new ArrayList<>(), true); Review Comment: [P2] Select an alias backing using the requested mapping type On ES5, backing `a` can contain an empty first type and a populated second type `wanted` with column `x`, while backing `b` contains a populated type `other` with column `y`. A legacy table configured with `type=wanted` previously selected `a`, then `wanted`. Probing with a null mapping type here skips `a` and chooses `b`; `getMappingProps` cannot find `wanted`, falls back to `other`, and `resolveFieldContext` rejects `x`, breaking the previously valid alias query. Thread the requested type into backing selection and cover an empty first type. Multiple types per index are part of the supported [ES5 mapping contract](https://www.elastic.co/guide/en/elasticsearch/reference/5.6/mapping.html). ########## fe/fe-core/src/main/java/org/apache/doris/datasource/es/EsMappingUtils.java: ########## @@ -0,0 +1,442 @@ +// 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.doris.datasource.es; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Set; + +/** + * ES mapping parsing and field resolution utilities. + */ +public final class EsMappingUtils { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + public static final Set<String> DEFAULT_DOCVALUE_DISABLED_FIELDS = + new HashSet<>(Arrays.asList("text", "annotated_text", "match_only_text")); + + private static final Set<String> ROOT_MAPPING_OBJECT_PARAMETERS = new HashSet<>(Arrays.asList( + "properties", "runtime", "_source", "_routing", "_field_names", "_size", "_data_stream_timestamp")); + + private static final Set<String> DOCVALUE_QUERY_TYPES = new HashSet<>(Arrays.asList( + "keyword", "byte", "short", "integer", "long", "unsigned_long", "float", "double", + "half_float", "scaled_float", "date", "date_nanos", "boolean", "ip", "geo_point")); + + private static final Set<String> IGNORE_MALFORMED_TYPES = new HashSet<>(Arrays.asList( + "byte", "short", "integer", "long", "unsigned_long", "float", "double", "half_float", "scaled_float", + "boolean", "date", "date_nanos", "ip", "geo_point", "geo_shape")); + + private EsMappingUtils() { + } + + /** + * Parse the top-level "mappings" node from an index mapping JSON response. + */ + public static ObjectNode getMapping(String indexMapping) { + try { + ObjectNode jsonNodes = (ObjectNode) MAPPER.readTree(indexMapping); + // Alias mapping response order is unspecified. A bare/typed empty backing can appear + // first; select a populated schema without discarding its _meta array annotations. + for (JsonNode index : jsonNodes) { + ObjectNode mapping = (ObjectNode) index.get("mappings"); + ObjectNode root = getRootSchema(mapping.deepCopy(), null, new ArrayList<>(), true); + if (root.path("properties").size() > 0) { + return mapping; + } + } + // Preserve the existing diagnostic when no backing index provides a usable schema. + return (ObjectNode) jsonNodes.iterator().next().get("mappings"); + } catch (IOException e) { + throw new DorisEsException("Failed to parse ES mapping: " + e.getMessage()); + } + } + + /** + * Extract the root schema from mapping node, handling ES 5.x/6.x/7.x/8.x differences. + */ + public static ObjectNode getRootSchema(ObjectNode mappings, String mappingType, + List<String> arrayFields) { + return getRootSchema(mappings, mappingType, arrayFields, false); + } + + private static ObjectNode getRootSchema(ObjectNode mappings, String mappingType, + List<String> arrayFields, boolean allowEmpty) { + if (mappingType == null) { + checkNonPropertiesFields(mappings, arrayFields, allowEmpty); + // Alias safety analysis must accept backing indices that have not acquired a mapping yet. + if (mappings.isEmpty()) { + return mappings; + } + String firstType = mappings.fieldNames().next(); + // A typeless mapping may contain only flags or root objects such as _source. + // Type wrappers are objects; legacy type names may also be names of scalar settings. + Iterator<String> keys = mappings.fieldNames(); + while (keys.hasNext()) { + String key = keys.next(); + JsonNode value = mappings.get(key); + // ES6 permits a type named runtime. Its properties contain field descriptors, + // whereas a runtime field named properties has scalar parameters such as type. + boolean legacyRuntime = "runtime".equals(key) && value.path("properties").isObject() + && allValuesAreObjects(value.path("properties")); + if (!value.isObject() || (ROOT_MAPPING_OBJECT_PARAMETERS.contains(key) && !legacyRuntime)) { + return mappings; + } + } + ObjectNode firstData = (ObjectNode) mappings.get(firstType); + checkNonPropertiesFields(firstData, arrayFields, allowEmpty); + return firstData; + } else { + if (mappings.has(mappingType)) { + ObjectNode jsonData = (ObjectNode) mappings.get(mappingType); + checkNonPropertiesFields(jsonData, arrayFields, allowEmpty); + return jsonData; + } + return getRootSchema(mappings, null, arrayFields, allowEmpty); + } + } + + private static boolean allValuesAreObjects(JsonNode node) { + for (JsonNode value : node) { + if (!value.isObject()) { + return false; + } + } + return true; + } + + /** + * Get mapping properties ObjectNode for a given index. + */ + public static ObjectNode getMappingProps(String sourceIndex, String indexMapping, + String mappingType) { + ObjectNode mappings = getMapping(indexMapping); + ObjectNode rootSchema = getRootSchema(mappings, mappingType, new ArrayList<>()); + ObjectNode properties = (ObjectNode) rootSchema.get("properties"); + if (properties == null) { + throw new DorisEsException( + "index[" + sourceIndex + "] type[" + mappingType + + "] mapping not found for the ES Cluster"); + } + return properties; + } + + /** + * Resolve field contexts (keyword sniff, doc_value, date compat) from mapping properties. + * + * @param columnNames column names to resolve + * @param sourceIndex index name for error messages + * @param indexMapping raw mapping JSON + * @param mappingType mapping type (null for ES 7+) + * @return populated EsFieldContext + */ + public static EsFieldContext resolveFieldContext(List<String> columnNames, + String sourceIndex, String indexMapping, String mappingType) { + EsFieldContext fieldContext = new EsFieldContext(); + ObjectNode properties = getMappingProps(sourceIndex, indexMapping, mappingType); + + // When columnNames is empty, resolve all fields from the mapping. + // This ensures column2typeMap, keyword sniff, docvalue, and date compat contexts + // are populated even when specific column handles are not provided. + Iterable<String> fieldsToResolve; + if (columnNames == null || columnNames.isEmpty()) { + List<String> allFields = new ArrayList<>(); + Iterator<String> iter = properties.fieldNames(); + while (iter.hasNext()) { + allFields.add(iter.next()); + } + fieldsToResolve = allFields; + } else { + fieldsToResolve = columnNames; + } + + for (String colName : fieldsToResolve) { + if ("_id".equals(colName)) { + continue; + } + if (!properties.has(colName)) { + throw new DorisEsException( + "index[" + sourceIndex + "] mapping[" + indexMapping + + "] not found column " + colName + " for the ES Cluster"); + } + ObjectNode fieldObject = (ObjectNode) properties.get(colName); + if (!fieldObject.has("type")) { + fieldContext.getColumn2typeMap().put(colName, "object"); + continue; + } + String fieldType = fieldObject.get("type").asText(); + fieldContext.getColumn2typeMap().put(colName, fieldType); + resolveDateFields(fieldContext, fieldObject, colName, fieldType); + resolveDocValuesFields(fieldContext, fieldObject, colName, fieldType); + } + resolveSourceValueSafety(fieldContext, indexMapping, mappingType, properties); + return fieldContext; + } + + private static void resolveSourceValueSafety(EsFieldContext context, String indexMapping, String mappingType, + ObjectNode schemaProperties) { + try { + // Mapping-induced source/index differences on any alias backing index must prevent + // filtering or replacing source values through that path. + JsonNode indices = MAPPER.readTree(indexMapping); + Set<String> columns = new HashSet<>(context.getColumn2typeMap().keySet()); + // Use the same populated schema as inference, including unprojected filter columns. + schemaProperties.fieldNames().forEachRemaining(columns::add); + // Filters can reference columns absent from the projection. Resolve their query paths + // from the exposed schema too; the projection still controls docvalue read eligibility. + for (String column : columns) { + ObjectNode field = (ObjectNode) schemaProperties.get(column); + resolveKeywordFields(context, field, column, field.path("type").asText()); + } + Set<String> unsafeDocValues = new HashSet<>(); + for (JsonNode index : indices) { + ObjectNode root = getRootSchema((ObjectNode) index.get("mappings"), mappingType, + new ArrayList<>(), true); + JsonNode mappingDefaults = index.path("settings").path("index").path("mapping"); + JsonNode properties = root.path("properties"); + if (isSourceDisabled(root.path("_source"))) { + // A source projection fails on disabled backings; full source preserves the + // enabled backings of a mixed alias while disabled hits remain NULL. + context.requireFullSourceRead(); + } + Set<String> copyTargets = new HashSet<>(); + collectCopyTargets(properties, copyTargets); + for (String column : columns) { + JsonNode field = properties.path(column); + JsonNode queryField = fieldAtPath(field, column, context.getFetchFieldsContext().get(column)); + JsonNode readField = fieldAtPath(field, column, context.getDocValueFieldsContext().get(column)); + // Disabled root parsing leaves source fields but produces no index/docvalues, + // even if an explicit properties section still describes those fields. + boolean sourceChanged = !root.path("enabled").asBoolean(true) + || filtersSourceColumn(root.path("_source"), column) + || root.path("runtime").has(column) || copyTargets.contains(column); + // ES mappings do not distinguish arrays from scalars. Even a short equality can + // match one indexed array element while Doris reads the entire source JSON string. + if (sourceChanged || changesSourceValue(field, mappingDefaults) + || changesSourceValue(queryField, mappingDefaults) + || "text".equals(queryField.path("type").asText()) || !isSearchable(queryField)) { + context.getSourceValueUnsafeFields().add(column); + } + // Source can omit generated/aliased values, retain unrounded numbers or decode + // default dates in local time. Docvalues remain eligible for these mappings. + if (requiresSourcePredicateEvaluation(field) || requiresSourcePredicateEvaluation(queryField)) { + context.getSourceModeUnsafeFields().add(column); + } + // Query and docvalue paths may select different multi-fields. An unused sibling's + // normalizer must not invalidate either path; only the chosen read value matters. + if (sourceChanged || changesSourceValue(field, mappingDefaults) + || changesSourceValue(readField, mappingDefaults) + || !readField.path("doc_values").asBoolean(true) + || DEFAULT_DOCVALUE_DISABLED_FIELDS.contains(readField.path("type").asText())) { + // One backing without this docvalue path would return missing values for its hits. + unsafeDocValues.add(column); + } + } + } + unsafeDocValues.forEach(context.getDocValueFieldsContext()::remove); + } catch (IOException e) { + throw new DorisEsException("Failed to parse ES mapping: " + e.getMessage()); + } + } + + private static JsonNode fieldAtPath(JsonNode field, String column, String path) { + return path == null || column.equals(path) ? field + : field.path("fields").path(path.substring(column.length() + 1)); + } + + private static boolean changesSourceValue(JsonNode field, JsonNode mappingDefaults) { + // With index-level ignore_malformed=true/coerce=false, source "5" survives without an + // indexed value. A field's explicit false overrides that default; keyword ignores it. + boolean ignoresMalformed = IGNORE_MALFORMED_TYPES.contains(field.path("type").asText()) + && field.path("ignore_malformed").asBoolean(mappingDefaults.path("ignore_malformed").asBoolean(false)); + return field.isMissingNode() || field.has("ignore_above") || field.hasNonNull("normalizer") + || field.hasNonNull("null_value") || ignoresMalformed; + } + + private static boolean requiresSourcePredicateEvaluation(JsonNode field) { + String type = field.path("type").asText(); + // Source dates use the session time zone, including epoch_millis in explicit formats, + // so their local value can differ from the UTC value used by an ES predicate. + return "scaled_float".equals(type) || "half_float".equals(type) || "alias".equals(type) Review Comment: [P2] Preserve numeric-token equivalence in source mode With `n:integer` and a projected normalized keyword `tag`, indexing `{"n":0.9999999999999999,"tag":"UPPER"}` stores integer 0 in ES. This PR removes the tag's docvalue eligibility and makes the whole scan read source. Pinned RapidJSON's normal-precision conversion rounds that numeric token to 1.0, so `get_int_value` now materializes `n=1`. Integer fields are absent from this safety guard, so `WHERE n=1` is still pushed and discards a row the source materializer exposes as 1; the old docvalue scan consistently read 0. ES source projection preserves this token. The same exposed flow also rounds FLOAT numeric token `1.0000000596046448` through double to `1.0`, while [ES8.19 directly parses FLOAT](https://github.com/elastic/elasticsearch/blob/v8.19.0/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java#L498-L501) as `1.0000001192092896`. Preserve equivalent numeric decoding or retain affected predicates locally; the quoted-string tests do not cover this case. The ES indexing conversion is visible in its [integer mapper](https://github.com/elastic/elasticsearch/blob/v8.3.3/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java#L815-L817) and [Jackson numeric conversion](https://github.com/FasterXML/jackson-core/blob/jackson-core-2.13.2/src/main/java/com/fasterxml/jackson/core/base/ParserBase.java#L939-L960). ########## fe/fe-core/src/main/java/org/apache/doris/datasource/es/source/EsScanNode.java: ########## @@ -456,4 +489,30 @@ private void buildQuery() throws UserException { } } + private static void splitConjuncts(Expr expr, List<Expr> conjuncts) { + if (expr instanceof CompoundPredicate + && ((CompoundPredicate) expr).getOp() == CompoundPredicate.Operator.AND) { + expr.getChildren().forEach(child -> splitConjuncts(child, conjuncts)); + } else { + conjuncts.add(expr); + } + } + + private void collectSourceUnsafeFields(Expr expr, Set<String> fields) { + if (expr instanceof FunctionCallExpr + && "esquery".equalsIgnoreCase(((FunctionCallExpr) expr).getFnName().getFunction())) { + return; + } + if (expr instanceof SlotRef) { + String name = ((SlotRef) expr).getColumnName(); + if (!"_id".equals(name) && table.getColumn(name) != null + && (table.getColumn(name).getType().isStringType() Review Comment: [P2] Keep JSONB null checks local in source mode With ES `obj:flattened` and a projected normalized keyword `tag`, `{"obj":{},"tag":"UPPER"}` now makes the whole scan use source. `EsUtil` maps flattened to JSONB and `ScrollParser` preserves `{}` as nonnull, but this guard marks only STRING/ARRAY fields. ES indexes no leaf for the empty object, so pushed `obj IS NULL` returns that nonnull object, and `obj IS NOT NULL` drops it. The previous docvalue scan saw the absent `obj` value as SQL NULL and was consistent. Include JSONB null predicates in source-mode safety and test both null checks with both scanners. The ES [empty-object parser](https://github.com/elastic/elasticsearch/blob/v8.3.3/server/src/main/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldParser.java#L68-L74) and [source projection](https://github.com/elastic/elasticsearch/blob/v8.3.3/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java#L338-L342) establish the differing representations. ########## be/src/exec/es/es_scroll_parser.cpp: ########## @@ -157,50 +149,306 @@ static const std::string INVALID_NULL_VALUE = return Status::RuntimeError(ss.str()); \ } while (false) +// ES integer coercion truncates decimal/scientific notation, e.g. "1.2e3" -> 1200. +// Shift the decimal point using digits so values near INT64_MAX never pass through double. template <typename T> -Status get_int_value(const rapidjson::Value& col, PrimitiveType type, void* slot, - bool pure_doc_value) { - if (col.IsNumber()) { - *reinterpret_cast<T*>(slot) = (T)(sizeof(T) < 8 ? col.GetInt() : col.GetInt64()); - return Status::OK(); +static T parse_es_integer(const char* text, size_t length, StringParser::ParseResult* result) { + text = skip_ascii_whitespaces(text, length); + size_t i = 0; + bool negative = length > 0 && text[0] == '-'; + if (length > 0 && (text[0] == '-' || text[0] == '+')) { + ++i; } - - if (pure_doc_value && col.IsArray() && !col.Empty()) { - RETURN_ERROR_IF_COL_IS_NOT_NUMBER(col[0], type); - *reinterpret_cast<T*>(slot) = (T)(sizeof(T) < 8 ? col[0].GetInt() : col[0].GetInt64()); - return Status::OK(); + size_t start = i; + int64_t digits = 0; + int64_t integer_digits = 0; + int64_t first_nonzero = -1; + bool dot = false; + for (; i < length; ++i) { + char c = text[i]; + if (c >= '0' && c <= '9') { + if (c != '0' && first_nonzero < 0) { + first_nonzero = digits; + } + ++digits; + integer_digits += !dot; + } else if (c == '.' && !dot) { + dot = true; + } else { + break; + } + } + size_t end = i; + int64_t exponent = 0; + bool negative_exponent = false; + if (i < length && (text[i] == 'e' || text[i] == 'E')) { + ++i; + if (i < length && (text[i] == '+' || text[i] == '-')) { + negative_exponent = text[i++] == '-'; + } + size_t exponent_start = i; + for (; i < length && text[i] >= '0' && text[i] <= '9'; ++i) { + // Saturation avoids overflow and work proportional to an untrusted exponent. + exponent = std::min(exponent * 10 + text[i] - '0', static_cast<int64_t>(length) + 64); + } + if (i == exponent_start) { + *result = StringParser::PARSE_FAILURE; + return 0; + } + } + if (digits == 0 || i != length) { + *result = StringParser::PARSE_FAILURE; + return 0; + } + int64_t count = integer_digits + (negative_exponent ? -exponent : exponent) - first_nonzero; + if (first_nonzero < 0 || count <= 0) { + *result = StringParser::PARSE_SUCCESS; + return 0; + } + // 39 digits cover every signed 128-bit value; the final parser checks the exact type bounds. + if (count > 39) { + *result = StringParser::PARSE_OVERFLOW; + return 0; + } + char integral[40]; + size_t size = 0; + if (negative) { + integral[size++] = '-'; + } + int64_t digit = 0; + for (i = start; i < end && count > 0; ++i) { + if (text[i] != '.' && digit++ >= first_nonzero) { + integral[size++] = text[i]; + --count; + } } + while (count-- > 0) { + integral[size++] = '0'; + } + return StringParser::string_to_int<T, true>(integral, size, result); +} - RETURN_ERROR_IF_COL_IS_ARRAY(col, type, true); - RETURN_ERROR_IF_COL_IS_NOT_STRING(col, type); +template <typename T> +static T parse_es_floating_string(const std::string& text, StringParser::ParseResult* result) { + // Java's floating parser also accepts hexadecimal exponents, f/d suffixes and ASCII trim + // characters. Keep the ordinary decimal fast path, and require the complete Java token. + std::string_view token(text); + while (!token.empty() && static_cast<unsigned char>(token.front()) <= 0x20) { + token.remove_prefix(1); + } + while (!token.empty() && static_cast<unsigned char>(token.back()) <= 0x20) { + token.remove_suffix(1); + } + auto magnitude = token; + if (!magnitude.empty() && (magnitude.front() == '+' || magnitude.front() == '-')) { + magnitude.remove_prefix(1); + } + if (magnitude == "NaN" || magnitude == "Infinity") { + return StringParser::string_to_float<T>(token.data(), token.size(), result); + } + if (!magnitude.empty() && + std::string_view("fFdD").find(magnitude.back()) != std::string_view::npos) { + token.remove_suffix(1); + magnitude.remove_suffix(1); + // Java permits outer whitespace, but not a gap between the number and its suffix. + if (magnitude.empty() || static_cast<unsigned char>(magnitude.back()) <= 0x20) { + *result = StringParser::PARSE_FAILURE; + return 0; + } + } + if (magnitude.empty() || + !((magnitude.front() >= '0' && magnitude.front() <= '9') || magnitude.front() == '.')) { + *result = StringParser::PARSE_FAILURE; + return 0; + } + const bool hexadecimal = magnitude.starts_with("0x") || magnitude.starts_with("0X"); + if (hexadecimal && magnitude.find_first_of("pP") == std::string_view::npos) { + *result = StringParser::PARSE_FAILURE; + return 0; + } + // Float.parseFloat rounds directly to float; an intermediate double can round a value + // just above a float midpoint back to that midpoint and then to the wrong float. + if (hexadecimal || std::is_same_v<T, float>) { + char* end = nullptr; + T number; + if constexpr (std::is_same_v<T, float>) { + number = std::strtof(token.data(), &end); + } else { + number = std::strtod(token.data(), &end); + } + *result = end == token.data() + token.size() ? StringParser::PARSE_SUCCESS + : StringParser::PARSE_FAILURE; + return number; + } + return StringParser::string_to_float<T>(token.data(), token.size(), result); +} - StringParser::ParseResult result; - const std::string& val = col.GetString(); - size_t len = col.GetStringLength(); - T v = StringParser::string_to_int<T>(val.c_str(), len, &result); - RETURN_ERROR_IF_PARSING_FAILED(result, col, type); +template <typename T> +static T parse_es_narrow_integer(const std::string& text, StringParser::ParseResult* result) { + // Unlike long's BigDecimal conversion, ES parses byte/short/integer strings through double: + // "0.99999999999999999" rounds to 1 before truncation. Keep long's exact path separate. + auto number = parse_es_floating_string<double>(text, result); + if (*result != StringParser::PARSE_SUCCESS) { + return 0; + } + // Java converts NaN to zero; casting it directly in C++ would be undefined. + if (std::isnan(number)) { + return 0; + } + using ParsedType = std::conditional_t<sizeof(T) == 1, int32_t, T>; + if (number < StringParser::numeric_limits<ParsedType>(true) || + number > StringParser::numeric_limits<ParsedType>(false)) { + *result = StringParser::PARSE_OVERFLOW; + return 0; + } + auto parsed = static_cast<ParsedType>(number); + // ES byte uses intValue() then checks the byte range, so "127.9" is valid. Short and + // integer instead check their own double bounds before casting (e.g. short "32767.9" fails). + if constexpr (sizeof(T) == 1) { + if (parsed < StringParser::numeric_limits<T>(true) || + parsed > StringParser::numeric_limits<T>(false)) { + *result = StringParser::PARSE_OVERFLOW; + return 0; + } + } + return static_cast<T>(parsed); +} - if (sizeof(T) < 16) { - *reinterpret_cast<T*>(slot) = v; +template <typename T> +Status get_int_value(const rapidjson::Value& col, PrimitiveType type, void* slot, + bool pure_doc_value) { + const rapidjson::Value* value = &col; + // A singleton source array has the same scalar value as its one docvalue. Keep rejecting + // multi-value source arrays: selecting an arbitrary element would discard source semantics. + if (col.IsArray() && !col.Empty() && (pure_doc_value || col.Size() == 1)) { + value = &col[0]; + } + RETURN_ERROR_IF_COL_IS_ARRAY((*value), type, true); + T parsed; + if (value->IsInt64() || value->IsUint64()) { + __int128 number = value->IsInt64() ? static_cast<__int128>(value->GetInt64()) + : static_cast<__int128>(value->GetUint64()); + if (number < StringParser::numeric_limits<T>(true) || + number > StringParser::numeric_limits<T>(false)) { + return Status::RuntimeError("ES integer value is out of range for {}", + type_to_string(type)); + } + parsed = static_cast<T>(number); + } else if (value->IsNumber()) { + // Jackson converts numeric tokens directly from double. Serializing INT64_MIN first + // rounds its text below the valid range; Java also saturates the rounded upper endpoint. + const double number = value->GetDouble(); + using ParsedType = std::conditional_t<(sizeof(T) < 4), int32_t, T>; + const auto maximum = static_cast<double>(StringParser::numeric_limits<ParsedType>(false)); + if (!std::isfinite(number) || + number < static_cast<double>(StringParser::numeric_limits<ParsedType>(true)) || + number > maximum) { + return Status::RuntimeError("ES integer value is out of range for {}", + type_to_string(type)); + } + const ParsedType converted = number == maximum + ? StringParser::numeric_limits<ParsedType>(false) + : static_cast<ParsedType>(number); + if constexpr (sizeof(T) < 4) { + if (converted < StringParser::numeric_limits<T>(true) || + converted > StringParser::numeric_limits<T>(false)) { + return Status::RuntimeError("ES integer value is out of range for {}", + type_to_string(type)); + } + } + parsed = static_cast<T>(converted); } else { - DCHECK(sizeof(T) == 16); - memcpy(slot, &v, sizeof(v)); + RETURN_ERROR_IF_COL_IS_NOT_STRING((*value), type); + std::string text(value->GetString(), value->GetStringLength()); + StringParser::ParseResult result; + if constexpr (sizeof(T) < 8) { + parsed = parse_es_narrow_integer<T>(text, &result); + } else { + parsed = parse_es_integer<T>(text.data(), text.size(), &result); + } + RETURN_ERROR_IF_PARSING_FAILED(result, (*value), type); } - + memcpy(slot, &parsed, sizeof(parsed)); return Status::OK(); } +// ES epoch_millis accepts signed decimal milliseconds with up to six fractional digits. +// Parse the integer part exactly: a double round trip can move a long timestamp across a second. +static bool parse_es_epoch_millis(std::string_view text, int64_t* seconds) { + bool negative = !text.empty() && text.front() == '-'; + size_t pos = negative ? 1 : 0; + size_t start = pos; + while (pos < text.size() && text[pos] >= '0' && text[pos] <= '9') { + ++pos; + } + size_t integer_end = pos; + // ES rejects empty numbers and integer parts wider than its signed-long grammar. + if (pos == start || pos - start > 19) { + return false; + } + bool nonzero_fraction = false; + if (pos < text.size() && text[pos] == '.') { + start = ++pos; + while (pos < text.size() && text[pos] >= '0' && text[pos] <= '9') { + nonzero_fraction |= text[pos++] != '0'; + } + // epoch_millis fractions express nanoseconds, so a seventh digit is invalid. + if (pos - start > 6) { + return false; + } + } + // Reject exponents, suffixes and trailing junk instead of accepting a numeric prefix. + if (pos != text.size()) { + return false; + } + StringParser::ParseResult result; + auto millis = StringParser::string_to_int<int64_t>(text.data(), integer_end, &result); + // Out-of-range milliseconds must not wrap into an otherwise valid calendar date. + if (result != StringParser::PARSE_SUCCESS) { + return false; + } + *seconds = millis / 1000; + // At second precision, -1000.5 ms belongs to 23:59:58, not 23:59:59. + if (negative && (millis % 1000 != 0 || nonzero_fraction)) { + --*seconds; + } + return true; +} + +template <typename DateValue> +static bool set_es_epoch_seconds(DateValue& value, int64_t seconds, + const cctz::time_zone& time_zone) { + const auto civil = + cctz::convert(cctz::time_point<cctz::seconds>(cctz::seconds(seconds)), time_zone); + // from_unixtime narrows the civil year without checking it; e.g. year 10000 must fail first. + if (civil.year() < 0 || civil.year() > MAX_YEAR) { + return false; + } + value.from_unixtime(seconds, time_zone); + return true; +} + template <PrimitiveType T> Status get_date_value_int(const rapidjson::Value& col, PrimitiveType type, bool is_date_str, typename PrimitiveTypeTraits<T>::CppType* slot, const cctz::time_zone& time_zone) { constexpr bool is_datetime_v1 = T == TYPE_DATE || T == TYPE_DATETIME; typename PrimitiveTypeTraits<T>::CppType dt_val; if (is_date_str) { - const std::string str_date = col.GetString(); - int str_length = col.GetStringLength(); + std::string str_date(col.GetString(), col.GetStringLength()); + size_t str_length = col.GetStringLength(); + // The default strict_date_optional_time parser wins over epoch_millis for a four-digit year. + // Thus "2024" means 2024-01-01, whereas "12345" means 12.345 seconds after the epoch. + if (str_length == 4 && str_date.find_first_not_of("0123456789") == std::string::npos) { + str_date += "-01-01"; + str_length = str_date.size(); + } bool success = false; - if (str_length > 19) { + int64_t epoch_seconds; + // Source fallback also reaches short, zero and fractional epoch strings, e.g. "0"/"1.5". + if (parse_es_epoch_millis(str_date, &epoch_seconds)) { Review Comment: [P2] Honor the mapped date grammar before trying epoch_millis For a legacy `ENGINE=ELASTICSEARCH` table with an explicit `DATETIMEV2` column over ES `format=basic_date` and `enable_docvalue_scan=false`, source `"20240101"` previously reached the compact-date cast and returned `2024-01-01 00:00:00`. The new unconditional epoch parser succeeds first and returns `1970-01-01 05:37:20`. Explicit legacy column types are preserved, so the catalog's STRING fallback for unknown formats does not prevent this path. Honor the mapping format/order when selecting the parser and add a custom-format case; [basic_date and epoch_millis have different grammars](https://www.elastic.co/guide/en/elasticsearch/reference/8.19/mapping-date-format.html). -- 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]
