Gabriel39 commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4398914351
##########
fe/fe-connector/fe-connector-es/src/main/java/org/apache/doris/connector/es/EsMappingUtils.java:
##########
@@ -150,12 +176,140 @@ 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)
Review Comment:
Fixed in b2f7d3fbbcfc9006aa7004306fd0011379b7897e.
Root enabled=false now marks the described source columns unsafe on every
backing even when explicit properties remain present, and removes their
unusable docvalue paths. The reported IS NULL false positive was reproduced
against live ES. New tests cover IS NULL/equality for both the populated
disabled root and an alias mixing it with an indexed backing; source values are
retained and filtering stays local.
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/EsScanPlanProvider.java:
##########
@@ -206,15 +211,13 @@ private ScanNodePropertiesResult buildScanNodeProperties(
// accepts any casing. Passing the raw string through let "TRUE" mean
https to FE and http to BE.
nodeProps.put(PROP_HTTP_SSL_ENABLED,
String.valueOf(props.isHttpSslEnabled()));
- // Query DSL (with not-pushed conjunct tracking)
- EsQueryDslResult dslResult = buildQueryDsl(filter, state);
- nodeProps.put(PROP_QUERY_DSL, dslResult.getQueryDsl());
+ // The read mode is scan-wide: losing a's docvalue path also changes a
keyword sibling b
+ // from scalar docvalues to source JSON (e.g. ["cat"]). Decide it
before pruning b's predicate.
+ int docValuesMode = useDocValueScan(columns, state);
Review Comment:
Fixed in b2f7d3fbbcfc9006aa7004306fd0011379b7897e.
Replaced decimal-point stripping with a shared exact integer coercion path
used by scalar fields and declared ARRAY elements. It applies decimal exponents
before truncation, bounds exponent work/output storage, and checks integer
bounds without a double round trip for integer strings. Singleton scalar
integer arrays are accepted; multi-value source arrays keep their existing
rejection. The old BE returned 1 for "1.2e3" and rejected [5] on both readers.
New unit/live tests cover those inputs, signs/fractions, INT64 precision
boundaries, narrow-type overflow and numeric-only docvalue 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]
