github-actions[bot] commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4400472641
##########
be/src/format/table/es/es_scroll_parser.cpp:
##########
@@ -723,15 +801,20 @@ Status ScrollParser::fill_columns(const
std::vector<SlotDescriptor*>& slots,
}
}
- const rapidjson::Value& col = (*line)[col_name];
-
auto type = slot_desc->type()->get_primitive_type();
-
- // when the column value is null, the subsequent type casting will
report an error
- if (col.IsNull() && slot_desc->is_nullable()) {
+ const rapidjson::Value& source_value = (*line)[col_name];
+ // Source fallback changes every sibling's decoder. A scalar
numeric/Boolean/date field
+ // may hold [value], [] or [null]; preserve its single value or
missing-value semantics.
+ // STRING keeps its JSON representation, and declared ARRAY keeps its
container.
+ bool scalar_source_array = !pure_doc_value && source_value.IsArray() &&
+ type != TYPE_ARRAY && !is_string_type(type);
Review Comment:
[P2] Preserve JSONB containers during scalar array normalization
`TYPE_JSONB` also satisfies this condition: `is_string_type` only includes
CHAR/VARCHAR/STRING. With docvalue scanning disabled, map `obj` as an
object/nested field and store `{"obj":[{"x":1}]}`. Both readers now pass only
`{"x":1}` to the JSONB branch; `obj:[]` and `obj:[null]` instead enter the SQL
NULL branch. The merge-base parser serialized each original JSON array intact.
This changes existing source reads without any predicate or newly forced
fallback. Restrict normalization to the intended scalar numeric/Boolean/date
families, or exclude JSONB, and add JSONB singleton/empty/null-element array
controls. This is distinct from the existing scalar numeric-array and declared
ARRAY predicate threads.
##########
be/src/format/table/es/es_scroll_parser.cpp:
##########
@@ -156,36 +146,153 @@ 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;
+ }
+ 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;
+ }
}
-
- 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 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);
-
- 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 = StringParser::string_to_float<double>(text.data(),
text.size(), 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 {
- DCHECK(sizeof(T) == 16);
- memcpy(slot, &v, sizeof(v));
+ if (!value->IsString() && !value->IsNumber()) {
+ RETURN_ERROR_IF_COL_IS_NOT_STRING((*value), type);
+ }
+ std::string text = value->IsString()
+ ? std::string(value->GetString(),
value->GetStringLength())
+ : json_value_to_string(*value);
Review Comment:
[P2] Preserve numeric INT64_MIN when converting double tokens
Map `n` as `long`, store the unquoted JSON number `-9223372036854775808.0`,
and disable docvalue scanning. [ES/Jackson numeric
conversion](https://github.com/FasterXML/jackson-core/blob/jackson-core-2.15.3/src/main/java/com/fasterxml/jackson/core/base/ParserBase.java#L996-L1013)
accepts this exactly representable double as INT64_MIN, and the merge-base
`insert_int_value` directly cast it successfully. This branch now reserializes
it: [the pinned RapidJSON
writer](https://github.com/Tencent/rapidjson/blob/232389d4f1012dddec4ef84861face2d2ba85709/include/rapidjson/internal/dtoa.h#L139-L150)
produces `-9223372036854776000.0`, which `parse_es_integer<int64_t>` rejects
as below INT64_MIN. Consequently a previously supported `SELECT n FROM idx`
fails on both readers. The same binary boundary survives ES source projection's
scientific serialization, so this does not depend on preserving the original
token text. Keep a checked numeric-double conversion path and reserve exact
decimal parsi
ng for strings; add an unquoted numeric INT64_MIN control. This numeric-token
defect differs from the earlier quoted-string exponent and narrow-string
rounding threads.
##########
be/src/format/table/es/es_scroll_parser.cpp:
##########
@@ -156,36 +146,153 @@ 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;
+ }
+ 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;
+ }
}
-
- 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 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);
-
- 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 = StringParser::string_to_float<double>(text.data(),
text.size(), result);
Review Comment:
[P2] Preserve the ES narrow-integer string grammar
This new helper uses the default `fast_float` grammar through
`StringParser`, requiring full input consumption. ES instead calls
[Double.parseDouble](https://github.com/elastic/elasticsearch/blob/v8.11.3/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/AbstractXContentParser.java#L98-L131),
whose [accepted
grammar](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Double.html#valueOf(java.lang.String))
includes hex floats and floating suffixes. With normalized keyword `a`,
integer `b`, source `{"a":"ok","b":"0x1.8p1"}` and docvalues enabled, ES
indexes 3 and the merge-base projection/current `SELECT b` read 3. The new
`SELECT a,b` source fallback fails because this parser stops after the leading
0; `"3d"` fails similarly. Match the accepted ES grammar while retaining narrow
rounding/bounds, and add controls. This is the new helper's grammar
restriction, distinct from the earlier decimal-rounding defect.
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsMappingUtils.java:
##########
@@ -150,12 +190,155 @@ public static EsFieldContext
resolveFieldContext(List<String> columnNames,
String fieldType = fieldObject.get("type").asText();
fieldContext.getColumn2typeMap().put(colName, fieldType);
resolveDateFields(fieldContext, fieldObject, colName, fieldType);
- resolveKeywordFields(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.
+ List<ObjectNode> roots = new ArrayList<>();
+ Set<String> columns = new
HashSet<>(context.getColumn2typeMap().keySet());
+ for (JsonNode index : MAPPER.readTree(indexMapping)) {
+ ObjectNode root = getRootSchema((ObjectNode)
index.get("mappings"), mappingType,
+ new ArrayList<>(), true);
+ roots.add(root);
+ }
+ // 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 (ObjectNode root : roots) {
+ 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) ||
changesSourceValue(queryField)
+ || "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) ||
changesSourceValue(readField)
+ || !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 DorisConnectorException("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) {
+ return field.isMissingNode() || field.has("ignore_above") ||
field.hasNonNull("normalizer")
+ || field.hasNonNull("null_value") ||
field.path("ignore_malformed").asBoolean(false);
Review Comment:
[P2] Include inherited malformed/coercion settings in predicate safety
`ignore_malformed` can be inherited from [index
settings](https://www.elastic.co/guide/en/elasticsearch/reference/8.19/ignore-malformed.html),
while `_mapping` omits the implicit field default. Set
`index.mapping.ignore_malformed=true` and `index.mapping.coerce=false`; map
normalized keyword `a` and plain `b:long`, then store `{"a":"ok","b":"5"}`. ES
accepts the document but omits `b` from indexing/docvalues because [coercion is
disabled](https://www.elastic.co/guide/en/elasticsearch/reference/8.19/coerce.html).
The new `SELECT a,b FROM idx WHERE b IS NULL` forces source, yet this
field-only check considers `b` safe and drops its residual after pushing
`must_not exists(b)`. The hit now materializes as `(ok,5)`, violating IS NULL;
the merge base read NULL from docvalues. Evaluate effective settings for every
backing, including field overrides, before authorizing source predicates. Add
this inherited-settings control; it is distinct from the earlier explicit
mapping-property and num
eric-decoder cases.
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsScanPlanProvider.java:
##########
@@ -206,15 +212,17 @@ private ScanNodePropertiesResult buildScanNodeProperties(
// accepts any casing. Passing the raw string through let "TRUE" mean
https to FE and http to BE.
nodeProps.put(PROP_HTTP_SSL_ENABLED,
String.valueOf(props.isHttpSslEnabled()));
- // Query DSL (with not-pushed conjunct tracking)
- EsQueryDslResult dslResult = buildQueryDsl(filter, state);
- nodeProps.put(PROP_QUERY_DSL, dslResult.getQueryDsl());
+ // The read mode is scan-wide: losing a's docvalue path also changes a
keyword sibling b
+ // from scalar docvalues to source JSON (e.g. ["cat"]). Decide it
before pruning b's predicate.
+ int docValuesMode = useDocValueScan(columns, state);
+ nodeProps.put(PROP_DOC_VALUES_MODE, String.valueOf(docValuesMode));
Review Comment:
[P2] Gate source plans that depend on the updated BE semantics
The new FE source fallback assumes this PR's BE decoder, but the provider
never sets `ScanNodePropertyKeys.REQUIRED_CURRENT_BACKEND_SEMANTICS`. An
available smooth-upgrade source BE remains eligible, and
`PluginDrivenScanNode.createScanRangeLocations` rejects it only when that
marker is present. For normalized keyword `a`, long `b`, source
`{"a":"ok","b":"1.2e3"}` and docvalues enabled, `Scan[a,b](b=1200)` now reads
source while pruning the numeric residual. A merge-base BE strips the decimal
suffix and returns `b=1`, violating WHERE; the new BE returns 1200. Old BEs
also ignore `read_full_source`. Declare the existing backend-generation
requirement for dependent source plans, or provide equivalent capability
handling. This is a mixed-generation scheduling defect beyond the already-fixed
homogeneous decoding threads; add smooth-upgrade coverage.
##########
be/src/format/table/es/es_scroll_parser.cpp:
##########
@@ -723,15 +801,20 @@ Status ScrollParser::fill_columns(const
std::vector<SlotDescriptor*>& slots,
}
}
- const rapidjson::Value& col = (*line)[col_name];
-
auto type = slot_desc->type()->get_primitive_type();
-
- // when the column value is null, the subsequent type casting will
report an error
- if (col.IsNull() && slot_desc->is_nullable()) {
+ const rapidjson::Value& source_value = (*line)[col_name];
+ // Source fallback changes every sibling's decoder. A scalar
numeric/Boolean/date field
+ // may hold [value], [] or [null]; preserve its single value or
missing-value semantics.
+ // STRING keeps its JSON representation, and declared ARRAY keeps its
container.
+ bool scalar_source_array = !pure_doc_value && source_value.IsArray() &&
+ type != TYPE_ARRAY && !is_string_type(type);
+ const rapidjson::Value& col =
+ scalar_source_array && source_value.Size() == 1 ?
source_value[0] : source_value;
+ bool is_null = col.IsNull() || (scalar_source_array &&
source_value.Empty());
Review Comment:
[P2] Preserve numeric empty-string NULL coercion in source fallback
An empty numeric string is supported by ES: with default coercion,
[NumberFieldMapper.value](https://github.com/elastic/elasticsearch/blob/v8.11.3/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java#L1636-L1647)
treats it as the default null value. Map normalized keyword `a` and ordinary
`b:long`, store `{"a":"ok","b":""}`, and enable docvalues. `SELECT a,b FROM
idx` previously read `(ok,NULL)`; the new normalizer safety check forces
source, but this null prepass misses `""`, so `parse_es_integer` fails with
zero digits and aborts. `SELECT b` still reads NULL from docvalues.
FLOAT/DOUBLE siblings also reject the empty string. Preserve numeric
empty-string-to-NULL materialization and add sibling/control cases across both
readers. This differs from Boolean empty-string=false and the prior non-null
numeric conversion threads.
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsMappingUtils.java:
##########
@@ -150,12 +190,155 @@ public static EsFieldContext
resolveFieldContext(List<String> columnNames,
String fieldType = fieldObject.get("type").asText();
fieldContext.getColumn2typeMap().put(colName, fieldType);
resolveDateFields(fieldContext, fieldObject, colName, fieldType);
- resolveKeywordFields(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.
+ List<ObjectNode> roots = new ArrayList<>();
+ Set<String> columns = new
HashSet<>(context.getColumn2typeMap().keySet());
+ for (JsonNode index : MAPPER.readTree(indexMapping)) {
+ ObjectNode root = getRootSchema((ObjectNode)
index.get("mappings"), mappingType,
+ new ArrayList<>(), true);
+ roots.add(root);
+ }
+ // 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 (ObjectNode root : roots) {
+ 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) ||
changesSourceValue(queryField)
+ || "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) ||
changesSourceValue(readField)
+ || !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);
Review Comment:
[P2] Handle nullable ARRAY elements before forcing source reads
With docvalues enabled, map normalized keyword `a`, `b:long`, and
`_meta.doris.array_fields:["b"]`; store `{"a":"ok","b":[null,1]}`. Removing
`a`'s path switches `SELECT a,b FROM idx` to source. The declared ARRAY
bypasses scalar normalization, and `parse_column -> process_column_array ->
get_int_value(null)` returns a type error. Previously ES omitted the null from
docvalues and the query read `[1]`; selecting `b` alone still works. ES
[supports null array
elements](https://www.elastic.co/guide/en/elasticsearch/reference/8.19/array.html).
The Boolean ARRAY variant reaches `GetString()` on a null node. Materialize
nullable members before element conversion and cover both readers. This
concerns element decoding, distinct from the existing ARRAY container-nullness
and inferred scalar-array threads.
--
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]
