Gabriel39 commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4398905954
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsScanPlanProvider.java:
##########
@@ -251,26 +254,57 @@ private void serializeFieldContexts(EsMetadataState
state, Map<String, String> n
}
private EsQueryDslResult buildQueryDsl(Optional<ConnectorExpression>
filter,
- EsMetadataState state) {
+ EsMetadataState state, int docValuesMode) {
if (!filter.isPresent()) {
return new EsQueryDslResult("{\"match_all\":{}}",
Collections.emptyList());
}
Map<String, String> fieldsContext = Collections.emptyMap();
List<String> needCompatDateFields = Collections.emptyList();
Map<String, String> column2typeMap = Collections.emptyMap();
+ Set<String> unsafeFields = new HashSet<>();
if (state.getFieldContext() != null) {
if (props.isKeywordSniff()) {
fieldsContext =
state.getFieldContext().getFetchFieldsContext();
}
needCompatDateFields =
state.getFieldContext().getNeedCompatDateFields();
column2typeMap = state.getFieldContext().getColumn2typeMap();
+
unsafeFields.addAll(state.getFieldContext().getSourceValueUnsafeFields());
+ if (docValuesMode == 0) {
+
unsafeFields.addAll(state.getFieldContext().getSourceModeUnsafeFields());
+ }
+ if (!props.isKeywordSniff()) {
+ // Without keyword sniff a term targets analyzed text, even
when a safe raw subfield
+ // exists. Do not apply the unused raw path's safety to the
actual text query.
+
unsafeFields.addAll(state.getFieldContext().getFetchFieldsContext().keySet());
+ }
+ }
+
+ if (docValuesMode == 0) {
+ collectSourceStringFields(filter.get(), unsafeFields);
}
return EsQueryDslBuilder.buildQueryDslWithResult(
filter.get(), fieldsContext, column2typeMap,
- props.isLikePushDown(), needCompatDateFields);
+ props.isLikePushDown(), needCompatDateFields, unsafeFields);
+ }
+
+ private static void collectSourceStringFields(ConnectorExpression
expression, Set<String> fields) {
+ if (expression instanceof ConnectorColumnRef) {
+ ConnectorColumnRef column = (ConnectorColumnRef) expression;
+ String type = column.getType().getTypeName();
+ // Source arrays are serialized as STRING, whereas ES predicates
match their elements.
+ // _id comes from hit metadata and cannot acquire an array
representation in source mode.
+ if (!"_id".equals(column.getColumnName())
+ && ("STRING".equalsIgnoreCase(type) ||
"VARCHAR".equalsIgnoreCase(type)
+ || "CHAR".equalsIgnoreCase(type))) {
Review Comment:
Fixed in b2f7d3fbbcfc9006aa7004306fd0011379b7897e.
Declared ARRAY references are now included in source-mode predicate safety.
Empty arrays remain nonnull containers in Doris, so source IS NULL/IS NOT NULL
stays local without an ES exists restriction. Added empty/nonempty/null/missing
ARRAY regressions across both readers and docvalue/parallel modes; FE tests
also verify that eligible docvalue-only ARRAY predicates retain their pushed
path.
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.
##########
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);
Review Comment:
Fixed in b2f7d3fbbcfc9006aa7004306fd0011379b7897e.
Default date mappings now require local predicates in source mode,
preserving date-only local-midnight decoding rather than filtering it using the
indexed UTC value. Docvalues remain eligible. The non-UTC live test verifies
both midnight and 08:00 source predicates and source/docvalue materialization
controls. The final control uses nullness rather than a date-literal pushdown
so it does not depend on the CI FE JVM timezone.
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]
