github-actions[bot] commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4386297844
##########
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");
Review Comment:
[P2] Restrict source-filter taint to fields the filter affects
The presence of any source include/exclude makes every column inexact. For
`_source: {"excludes":["a"]}` with ordinary keyword fields `a` and `b`, the
source and indexed values of `b` are still identical. This code nevertheless
marks `b` unsafe and removes its bounded-keyword exemption, so a previously
supported query such as `b = 'ok' OR esquery(b, '{"term":{"b":"other"}}')` now
fails FE planning with `Cannot evaluate esquery locally`. Both branches can
still be evaluated exactly by ES; only the unrelated field `a` was excluded.
This differs from the earlier mixed-native thread, which involved a genuinely
affected field. Determine source filtering per field, including include/exclude
patterns, and cover native Boolean queries on a field preserved by the source
filter.
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsQueryDslBuilder.java:
##########
@@ -143,18 +168,85 @@ public static EsQueryDslResult
buildQueryDslWithResult(ConnectorExpression filte
return new EsQueryDslResult(queryDsl, notPushedIndices);
}
+ // Dropping a source-value predicate after translating a
transformed/missing index value
+ // cannot be repaired by a residual: matching source documents may
already be absent.
+ if (referencesUnsafeSourceField(filter, sourceValueUnsafeFields,
keywordIgnoreAboveLimits)) {
+ requireLocalEvaluation(filter);
+ return new EsQueryDslResult(matchAllQuery(),
Collections.singletonList(0));
+ }
// Single expression (not AND)
List<ConnectorExpression> notPushDownList = new ArrayList<>();
String dsl = toEsDsl(filter, notPushDownList, fieldsContext,
likePushDown, needCompatDateFields, column2typeMap);
// A nested AND can return a partial DSL while recording an
unsupported child. OR/NOT must
// remain local in that case: e.g. NOT(a = 1 AND local_predicate)
cannot become NOT(a = 1).
if (dsl == null || !notPushDownList.isEmpty()) {
+ requireLocalEvaluation(filter);
return new EsQueryDslResult(matchAllQuery(),
Collections.singletonList(0));
}
return new EsQueryDslResult(dsl, Collections.emptyList());
}
+ private static boolean referencesUnsafeSourceField(ConnectorExpression
expression, Set<String> unsafeFields,
+ Map<String, Integer> keywordIgnoreAboveLimits) {
+ if (isExactKeywordPredicate(expression, keywordIgnoreAboveLimits)) {
+ return false;
+ }
+ if (expression instanceof ConnectorFunctionCall
+ && "esquery".equals(((ConnectorFunctionCall)
expression).getFunctionName())) {
+ // esquery explicitly requests ES index semantics rather than SQL
source-value semantics.
+ return false;
+ }
+ if (expression instanceof ConnectorColumnRef) {
+ return unsafeFields.contains(((ConnectorColumnRef)
expression).getColumnName());
+ }
+ for (ConnectorExpression child : expression.getChildren()) {
+ if (referencesUnsafeSourceField(child, unsafeFields,
keywordIgnoreAboveLimits)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean isExactKeywordPredicate(ConnectorExpression
expression, Map<String, Integer> limits) {
+ if (expression instanceof ConnectorComparison) {
+ ConnectorComparison comparison = (ConnectorComparison) expression;
+ if (comparison.getOperator() != ConnectorComparison.Operator.EQ
+ && comparison.getOperator() !=
ConnectorComparison.Operator.EQ_FOR_NULL) {
+ return false;
+ }
+ return isBoundedKeywordLiteral(comparison.getLeft(),
comparison.getRight(), limits)
+ || isBoundedKeywordLiteral(comparison.getRight(),
comparison.getLeft(), limits);
+ }
+ if (expression instanceof ConnectorIn) {
+ ConnectorIn in = (ConnectorIn) expression;
+ return !in.isNegated() && in.getInList().stream()
+ .allMatch(value -> isBoundedKeywordLiteral(in.getValue(),
value, limits));
+ }
+ return false;
+ }
+
+ private static boolean isBoundedKeywordLiteral(ConnectorExpression column,
ConnectorExpression value,
+ Map<String, Integer> limits) {
+ if (!(column instanceof ConnectorColumnRef) || !(value instanceof
ConnectorLiteral)) {
+ return false;
+ }
+ Integer limit = limits.get(((ConnectorColumnRef)
column).getColumnName());
+ Object literal = ((ConnectorLiteral) value).getValue();
+ return limit != null && literal instanceof String && ((String)
literal).length() <= limit;
Review Comment:
[P2] Preserve source-array semantics in bounded keyword predicates
A length bound does not establish scalar equality. With `a` mapped as
`{"type":"keyword","ignore_above":4,"doc_values":true}` and source
`a=["cat","elephant"]` (without `array_fields` metadata), Doris infers STRING.
The new safety pass removes its docvalue entry, so both readers now return the
JSON string `["cat","elephant"]`; nevertheless this check exempts `a = 'cat'`
and positive IN from the residual. ES matches the `cat` array member and the
engine prunes the predicate, so `SELECT a FROM idx WHERE a = 'cat'` returns a
value that fails that equality. Previously the explicit docvalue path returned
the scalar `cat`, since the longer member was ignored. ES applies [ignore_above
per array
element](https://www.elastic.co/guide/en/elasticsearch/reference/8.19/ignore-above.html),
and `ScrollParser::fill_columns` explicitly serializes source arrays as JSON.
Keep these source predicates local unless scalar equivalence is established,
and add array-valued STRING cases across both readers
/docvalue modes.
##########
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:
[P2] Preserve queries on doc-value-only keyword fields
`index=false` does not necessarily make a field unsearchable or change its
source value. ES8 supports [queries on doc-value-only keyword
fields](https://www.elastic.co/guide/en/elasticsearch/reference/8.19/doc-values.html).
For `a: {"type":"keyword","index":false,"doc_values":true}` with scalar source
`a="cat"`, no normalizer or source filtering, equality remains exact and worked
before this change. This condition nevertheless marks `a` inexact/unsafe and
removes its keyword exemption, so `a = 'cat' OR esquery(b,
'{"term":{"b":"other"}}')` now fails FE planning with `Cannot evaluate esquery
locally` despite both branches being supported remotely. Distinguish genuinely
unsearchable fields from doc-value-only searchable fields when deriving
exactness, and cover this supported native-OR configuration. The existing
index-disabled test only checks local fallback and misses the native Boolean
regression.
##########
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;
Review Comment:
[P2] Check the selected keyword path before rejecting native queries
This loop treats a transformation on an unused keyword subfield as a
transformation of the path Doris actually queries. With keyword sniff enabled,
let text field `a` have `aa_folded:
{"type":"keyword","normalizer":"lowercase_value"}` followed by `zz_raw:
{"type":"keyword"}` (with the normalizer defined in the index settings). Both
`resolveKeywordFields` and `resolveDocValuesFields` select `a.zz_raw`, which
preserves scalar source values. Nevertheless the unused `aa_folded` sets
`inexact=true`, removes `a`'s exact-keyword bound and makes `a = 'cat' OR
esquery(b, '{"term":{"b":"other"}}')` fail FE planning. Before this change both
branches were pushed through the unchanged selected paths. Determine predicate
safety from the selected keyword path and read safety from the selected
docvalue path; an unrelated alternative subfield should not invalidate them.
Add a native-OR test with a safe selected subfield and an unused normalized
sibling.
--
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]
