924060929 commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4398212461
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsMappingUtils.java:
##########
@@ -153,9 +162,101 @@ public static EsFieldContext
resolveFieldContext(List<String> columnNames,
resolveKeywordFields(fieldContext, fieldObject, colName,
fieldType);
resolveDocValuesFields(fieldContext, fieldObject, colName,
fieldType);
}
+ resolveSourceValueSafety(fieldContext, indexMapping, mappingType);
return fieldContext;
}
+ private static void resolveSourceValueSafety(EsFieldContext context,
String indexMapping, String mappingType) {
+ try {
+ // An alias can span mappings. Every backing index must preserve
source semantics before
+ // a predicate may remove rows or doc_values may replace the value
returned to Doris.
+ 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);
+ }
+ // Only the first mapping's schema is exposed by Doris, including
unprojected filter columns.
+
roots.get(0).path("properties").fieldNames().forEachRemaining(columns::add);
+ Set<String> inexactKeywords = new HashSet<>();
+ for (ObjectNode root : roots) {
+ JsonNode properties = root.path("properties");
+ JsonNode source = root.path("_source");
+ boolean filteredSource =
!source.path("enabled").asBoolean(true)
+ || source.has("includes") || source.has("excludes");
+ Set<String> copyTargets = new HashSet<>();
+ collectCopyTargets(properties, copyTargets);
+ for (String column : columns) {
+ JsonNode field = properties.path(column);
+ String indexedPath =
context.getFetchFieldsContext().get(column);
+ JsonNode indexedField = field;
+ if (indexedPath != null) {
+ indexedField =
field.path("fields").path(indexedPath.substring(column.length() + 1));
+ }
+ boolean inexact = filteredSource ||
root.path("runtime").has(column)
+ || copyTargets.contains(column) ||
changesSourceValueExceptLength(field)
+ || changesSourceValueExceptLength(indexedField);
+ for (JsonNode subfield : field.path("fields")) {
+ if ("keyword".equals(subfield.path("type").asText())
+ && changesSourceValueExceptLength(subfield)) {
+ inexact = true;
+ }
+ }
+ if (inexact || field.has("ignore_above") ||
indexedField.has("ignore_above")
+ || hasUnsafeKeywordSubfield(field)) {
+ context.getSourceValueUnsafeFields().add(column);
+ }
+ if (!inexact &&
"keyword".equals(indexedField.path("type").asText())) {
+ // Equality/IN with bounded literals stay exact:
ignored longer values cannot
+ // equal those literals. Intersect the bound over
every backing index.
+ context.getKeywordIgnoreAboveLimits().merge(column,
+
indexedField.path("ignore_above").asInt(Integer.MAX_VALUE), Math::min);
+ } else {
+ inexactKeywords.add(column);
+ }
+ }
+ }
+
inexactKeywords.forEach(context.getKeywordIgnoreAboveLimits()::remove);
+ } catch (IOException e) {
+ throw new DorisConnectorException("Failed to parse ES mapping: " +
e.getMessage());
+ }
+ // Local residuals must read the original value, including with
explicit doc_values=true.
+
context.getSourceValueUnsafeFields().forEach(context.getDocValueFieldsContext()::remove);
+ }
+
+ private static boolean hasUnsafeKeywordSubfield(JsonNode field) {
+ for (JsonNode subfield : field.path("fields")) {
+ if ("keyword".equals(subfield.path("type").asText()) &&
changesSourceValue(subfield)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean changesSourceValue(JsonNode field) {
+ return field.has("ignore_above") ||
changesSourceValueExceptLength(field);
+ }
+
+ private static boolean changesSourceValueExceptLength(JsonNode field) {
+ return field.isMissingNode() || field.has("normalizer")
+ || field.has("null_value") ||
field.path("ignore_malformed").asBoolean(false)
+ || !field.path("index").asBoolean(true);
Review Comment:
Rechecked at `0a792c9461252986de54efcf38bd0d7d845c0612`: the keyword case is
fixed, but this restriction still breaks doc-value-only numeric fields. For `a:
{"type":"long","index":false,"doc_values":true}` and an ordinary keyword `b`,
with docvalue scanning enabled:
```sql
SELECT a FROM idx
WHERE a = 1 OR esquery(b, '{"term":{"b":"ok"}}');
```
`EsMappingUtils.isSearchable` (lines 254-255) only permits the doc-values
exception for keyword, so `a` enters `sourceValueUnsafeFields` even while its
docvalue read path remains eligible. `buildQueryDslWithResult` then rejects
this whole OR via `requireLocalEvaluation`, reporting `Cannot evaluate esquery
locally`. At the merge base, both branches translate remotely. Elasticsearch
supports queries on doc-value-only numeric, date and boolean fields as well:
https://www.elastic.co/guide/en/elasticsearch/reference/8.19/doc-values.html#doc-values-index-only
Please keep searchability separate from source/index value equivalence and
account for supported doc-value-only field types. Extend the existing keyword
control with a BIGINT column and native OR/NOT. This is a statically traced
planning regression; I did not run a live reproduction in this review.
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsMappingUtils.java:
##########
@@ -153,9 +153,73 @@ public static EsFieldContext
resolveFieldContext(List<String> columnNames,
resolveKeywordFields(fieldContext, fieldObject, colName,
fieldType);
resolveDocValuesFields(fieldContext, fieldObject, colName,
fieldType);
}
+ resolveSourceValueSafety(fieldContext, indexMapping, mappingType);
return fieldContext;
}
+ private static void resolveSourceValueSafety(EsFieldContext context,
String indexMapping, String mappingType) {
+ try {
+ // An alias can span mappings. Every backing index must preserve
source semantics before
+ // a predicate may remove rows or doc_values may replace the value
returned to Doris.
+ for (JsonNode index : MAPPER.readTree(indexMapping)) {
+ ObjectNode root = getRootSchema((ObjectNode)
index.get("mappings"), mappingType, new ArrayList<>());
Review Comment:
Rechecked at `0a792c9461252986de54efcf38bd0d7d845c0612`: `{}`, typed empty
mappings and metadata-only mappings are handled, but a supported
configuration-only backing still fails. Consider an alias mapping response in
this order:
```json
{"populated":{"mappings":{"properties":{"a":{"type":"long"}}}},"disabled":{"mappings":{"enabled":false}}}
```
The merge-base schema/scan resolution uses the populated first mapping, so
this was readable with parallel scroll disabled. The new every-backing loop in
`resolveSourceValueSafety` (lines 188-190) sends the second mapping through
`getRootSchema`. `checkNonPropertiesFields` leaves `enabled`; with no
`properties`, line 96 casts the BooleanNode `false` to ObjectNode and raises
ClassCastException before scanning. Reversing the response order also fails in
the new populated-mapping selection loop. A top-level `enabled:false` is a
valid Elasticsearch mapping:
https://www.elastic.co/guide/en/elasticsearch/reference/8.19/enabled.html
Please distinguish typeless mapping configuration from a typed mapping
rather than treating its first arbitrary member as a type. Add both backing
orders to the empty-backing tests. This is an FE query failure, not a BE crash;
the path was verified statically, without a live ES run.
--
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]
