Gabriel39 commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4398911132
##########
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);
Review Comment:
Fixed in b2f7d3fbbcfc9006aa7004306fd0011379b7897e.
The shared source decoder now treats an empty Boolean string as false in
both scalar and ARRAY-element paths. Nonempty values still use the existing
parser and invalid values still fail. The old BE failed the new scalar/ARRAY
unit tests and live sibling-fallback query; the updated ASAN BE passes those
tests and the full reader/docvalue/parallel SQL matrix, including a
Boolean-only docvalue control.
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:
##########
@@ -66,25 +80,37 @@ public static ObjectNode getMapping(String indexMapping) {
*/
public static ObjectNode getRootSchema(ObjectNode mappings, String
mappingType,
List<String> arrayFields) {
+ return getRootSchema(mappings, mappingType, arrayFields, false);
+ }
+
+ private static ObjectNode getRootSchema(ObjectNode mappings, String
mappingType,
+ List<String> arrayFields, boolean allowEmpty) {
if (mappingType == null) {
- checkNonPropertiesFields(mappings, arrayFields);
- String firstType = mappings.fieldNames().next();
- if (StreamSupport.stream(Spliterators.spliteratorUnknownSize(
- mappings.fieldNames(), Spliterator.ORDERED), false)
- .anyMatch(s -> s.contains("properties"))) {
+ checkNonPropertiesFields(mappings, arrayFields, allowEmpty);
+ // Alias safety analysis must accept backing indices that have not
acquired a mapping yet.
+ if (mappings.isEmpty()) {
return mappings;
- } else {
- ObjectNode firstData = (ObjectNode) mappings.get(firstType);
- checkNonPropertiesFields(firstData, arrayFields);
- return firstData;
}
+ String firstType = mappings.fieldNames().next();
+ // A typeless mapping may contain only flags or root objects such
as _source.
+ // Type wrappers are objects; legacy type names may also be names
of scalar settings.
+ Iterator<String> keys = mappings.fieldNames();
+ while (keys.hasNext()) {
+ String key = keys.next();
+ if (!mappings.get(key).isObject() ||
ROOT_MAPPING_OBJECT_PARAMETERS.contains(key)) {
Review Comment:
Fixed in b2f7d3fbbcfc9006aa7004306fd0011379b7897e.
Legacy runtime type wrappers are now distinguished by their properties
structure, rather than rejected solely by the name runtime. Extended
schema-inference and field-context tests with that ES6 type name, and directly
checked a typeless runtime-only mapping whose runtime field is itself named
properties. The previous connector failed the legacy schema unit test; both
shapes now pass. This compatibility case was validated with mapping-response
unit tests, not a live ES6 server.
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]
