github-actions[bot] commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4399172770
##########
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] Keep floating-point singleton arrays readable after source fallback
With docvalues enabled, map `a` as a keyword with a configured lowercase
normalizer and `b` as ordinary `double`, then insert `{"a":"ok","b":[1.5]}`
without `array_fields` metadata. Removing `a` here switches `SELECT a,b FROM
idx` to source mode. Both readers then call
`ScrollParser::insert_float_value<double>`, whose array unwrapping requires
`pure_doc_value`; the source array reaches `RETURN_ERROR_IF_COL_IS_ARRAY` and
aborts the query. The merge-base projection and the `SELECT b` docvalue control
read 1.5 successfully. [Elasticsearch supports arrays for numeric
fields](https://www.elastic.co/guide/en/elasticsearch/reference/8.19/array.html).
This is the separate FLOAT/DOUBLE decoder, distinct from the existing
integer/Boolean decoder threads and scaled/half-float predicate-rounding
findings; no WHERE clause is needed. Preserve compatible floating
singleton/empty/null-array materialization, or a compatible read path, and
cover both readers with the docvalue control.
##########
be/src/format/table/es/es_scroll_parser.cpp:
##########
@@ -156,36 +145,117 @@ 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;
Review Comment:
[P2] Match narrow integer coercion at rounding boundaries
The new exact decimal truncation is also used for INT/SMALLINT/TINYINT, but
ES does not coerce their strings like `long`: its [intValue/shortValue
implementation](https://github.com/elastic/elasticsearch/blob/v8.11.3/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/AbstractXContentParser.java)
parses through `Double.parseDouble` before casting. For `b:integer` with
source `"0.99999999999999999"`, ES indexes 1; this helper has
`integer_digits=1`, `first_nonzero=1`, and returns 0.
With a normalized keyword sibling `a`, source
`{"a":"ok","b":"0.99999999999999999"}` and docvalues enabled, the new fallback
makes `SELECT a,b FROM idx WHERE b=1` return `(ok,0)`: plain integer `b`
remains pushable and its residual is pruned. The merge base and `SELECT b`
control read 1. This is a new narrow-type assumption in the shared helper; the
earlier long exponent example is fixed. Preserve each ES integer family's
coercion semantics and add narrow rounding-boundary controls across both
readers.
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsQueryDslBuilder.java:
##########
@@ -307,6 +356,32 @@ private static String notToDsl(ConnectorNot not,
boolean likePushDown,
List<String> needCompatDateFields,
Map<String, String> column2typeMap) {
+ // Optimizer NOT simplification is optional. Inverting a comparison
keeps NULL as
+ // UNKNOWN; complementing its ES true-set would instead make missing
values match.
+ ConnectorExpression operand = not.getOperand();
+ ConnectorExpression normalized = null;
+ if (operand instanceof ConnectorNot) {
+ normalized = ((ConnectorNot) operand).getOperand();
+ } else if (operand instanceof ConnectorComparison) {
+ ConnectorComparison comparison = (ConnectorComparison) operand;
+ ConnectorComparison.Operator inverse =
negateOperator(comparison.getOperator());
+ if (inverse != null) {
+ normalized = new ConnectorComparison(inverse,
comparison.getLeft(), comparison.getRight());
Review Comment:
[P2] Preserve multivalue semantics in retained range negation
With docvalues enabled and
`disable_nereids_expression_rules='SIMPLIFY_NOT_EXPR'`, map `a:long`,
`b:keyword`, omit `array_fields`, and store `{"a":[1,2],"b":"miss"}`. The query
`SELECT a FROM idx WHERE NOT(a<2) OR esquery(b,'{"term":{"b":"hit"}}')` retains
this scan filter:
```text
Scan[a](OR(NOT(LT(a,2)), esquery(b,term hit)))
```
`ExpressionTranslator.visitNot` preserves range NOT. Previously
`must_not(range a<2)` excluded this document. The new inversion emits `range
a>=2`, which matches member 2; the connector prunes the residual, while
`get_int_value` reads the first [naturally ordered numeric
docvalue](https://www.elastic.co/docs/reference/elasticsearch/mapping-reference/doc-values#multi-valued-doc-values-ordering),
returning 1. That row fails `NOT(1<2) OR false`.
This is distinct from the existing NULL/NE thread: it changes non-NULL
multivalue range matching in pure docvalue mode. Establish scalar equivalence
or retain safe evaluation instead of treating an inverse existential range as
the complement. Add this retained-NOT array case beside the scalar/NULL
controls.
--
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]
