Gabriel39 commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4398909354


##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsMappingUtils.java:
##########
@@ -150,12 +169,139 @@ 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");
+                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));
+                    boolean sourceChanged = 
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);
+                    }
+                    // These mappings round indexed numbers while _source 
retains the original value.
+                    // Keep docvalues eligible: only a final source read makes 
their predicates unsafe.
+                    if (roundsNumericSourceValue(field) || 
roundsNumericSourceValue(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);
+    }
+
+    private static boolean roundsNumericSourceValue(JsonNode field) {
+        String type = field.path("type").asText();
+        return "scaled_float".equals(type) || "half_float".equals(type);
+    }
+
+    private static boolean isSearchable(JsonNode field) {
+        // Disabling the inverted index does not change the value. ES can 
still search keyword
+        // docvalues; only a field with neither representation needs a local 
predicate.
+        return field.path("index").asBoolean(true)
+                || ("keyword".equals(field.path("type").asText()) && 
field.path("doc_values").asBoolean(true));
+    }
+
+    private static boolean filtersSourceColumn(JsonNode source, String column) 
{
+        JsonNode includes = source.path("includes");
+        return !source.path("enabled").asBoolean(true)

Review Comment:
   Fixed in b2f7d3fbbcfc9006aa7004306fd0011379b7897e.
   
   Recognized both source.enabled=false and source.mode=disabled on every 
backing. Live reproduction additionally exposed ES rejecting projected-source 
requests with HTTP 400. For these source reads only, an optional flag in the 
existing ES properties map makes BE request _source:true. Disabled hits remain 
missing/NULL, and enabled backings in a mixed alias retain their source. Tests 
cover predicates and full rows for each encoding, standalone indices and mixed 
aliases, plus default projection/docvalue request controls.
   
   Validation: 158 FE ES tests passed (one optional ES9 class skipped), 12 BE 
unit tests passed, and four live ES8 suites passed 1,452 stored checks and 
1,356 independent comparisons. The final new fixture also passed a separate 
non-generation rerun. FE Checkstyle, clang-format 16, clang-tidy, build hygiene 
and the standard ASAN BE build passed.



-- 
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]

Reply via email to