Skip to content

Commit 4ddd084

Browse files
authored
Fix incorrect defaults in FieldStorageResolver. (opensearch-project#21408)
* Fix incorrect defaults in FieldStorageResolver. Signed-off-by: Marc Handalian <marc.handalian@gmail.com> * test fixes Signed-off-by: Marc Handalian <marc.handalian@gmail.com> --------- Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
1 parent 4432538 commit 4ddd084

4 files changed

Lines changed: 77 additions & 5 deletions

File tree

sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DataFusionAnalyticsBackendPlugin.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public class DataFusionAnalyticsBackendPlugin implements AnalyticsSearchBackendP
4444
SUPPORTED_FIELD_TYPES.addAll(FieldType.keyword());
4545
SUPPORTED_FIELD_TYPES.addAll(FieldType.date());
4646
SUPPORTED_FIELD_TYPES.add(FieldType.BOOLEAN);
47+
SUPPORTED_FIELD_TYPES.add(FieldType.TEXT);
4748
}
4849

4950
private static final Set<FilterOperator> STANDARD_FILTER_OPS = Set.of(

sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/FieldStorageResolver.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ public List<FieldStorageInfo> resolve(List<String> fieldNames) {
9292
}
9393

9494
private static FieldStorageInfo resolveField(String fieldName, String fieldType, Map<String, Object> fieldProps, String primaryFormat) {
95-
// Doc values: present for all types except text, unless explicitly disabled
96-
boolean hasDocValues = !"text".equals(fieldType) && !Boolean.FALSE.equals(fieldProps.get("doc_values"));
95+
// Doc values: present for all types unless explicitly disabled
96+
boolean hasDocValues = !Boolean.FALSE.equals(fieldProps.get("doc_values"));
9797

98-
// Index: only when explicitly set to true in mapping
99-
boolean isIndexed = Boolean.TRUE.equals(fieldProps.get("index"));
98+
// Index: only when explicitly set to false in mapping - enabled by default.
99+
boolean isIndexed = !Boolean.FALSE.equals(fieldProps.get("index"));
100100

101101
// Stored fields: only when explicitly set to true in mapping
102102
boolean isStored = Boolean.TRUE.equals(fieldProps.get("store"));
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.analytics.planner;
10+
11+
import org.opensearch.cluster.metadata.IndexMetadata;
12+
import org.opensearch.cluster.metadata.MappingMetadata;
13+
import org.opensearch.common.settings.Settings;
14+
import org.opensearch.core.index.Index;
15+
import org.opensearch.test.OpenSearchTestCase;
16+
17+
import java.util.List;
18+
import java.util.Map;
19+
20+
import static org.mockito.Mockito.mock;
21+
import static org.mockito.Mockito.when;
22+
23+
/**
24+
* Unit tests for {@link FieldStorageResolver} field storage resolution.
25+
*/
26+
public class FieldStorageResolverTests extends OpenSearchTestCase {
27+
28+
public void testTextFieldGetsDocValuesInPrimaryFormat() {
29+
FieldStorageResolver resolver = newResolver("parquet", Map.of("name", Map.of("type", "text")));
30+
31+
FieldStorageInfo info = resolver.resolve(List.of("name")).get(0);
32+
33+
assertEquals("name", info.getFieldName());
34+
assertEquals(List.of("parquet"), info.getDocValueFormats());
35+
assertEquals(List.of("lucene"), info.getIndexFormats());
36+
}
37+
38+
public void testLongFieldGetsDocValuesInPrimaryFormat() {
39+
FieldStorageResolver resolver = newResolver("parquet", Map.of("age", Map.of("type", "long")));
40+
41+
FieldStorageInfo info = resolver.resolve(List.of("age")).get(0);
42+
43+
assertEquals("age", info.getFieldName());
44+
assertEquals(List.of("parquet"), info.getDocValueFormats());
45+
assertEquals(List.of("lucene"), info.getIndexFormats());
46+
}
47+
48+
public void testFieldWithAllStorageDisabledHasNoStorage() {
49+
IllegalStateException ex = expectThrows(
50+
IllegalStateException.class,
51+
() -> newResolver("parquet", Map.of("name", Map.of("type", "text", "doc_values", false, "index", false)))
52+
);
53+
assertTrue("expected 'no storage' error, got: " + ex.getMessage(), ex.getMessage().contains("has no storage in any format"));
54+
}
55+
56+
private static FieldStorageResolver newResolver(String primaryFormat, Map<String, Map<String, Object>> fieldMappings) {
57+
Map<String, Object> mappingSource = Map.of("properties", fieldMappings);
58+
59+
MappingMetadata mappingMetadata = mock(MappingMetadata.class);
60+
when(mappingMetadata.sourceAsMap()).thenReturn(mappingSource);
61+
62+
IndexMetadata indexMetadata = mock(IndexMetadata.class);
63+
when(indexMetadata.getIndex()).thenReturn(new Index("test_index", "uuid"));
64+
when(indexMetadata.getSettings()).thenReturn(Settings.builder().put("index.composite.primary_data_format", primaryFormat).build());
65+
when(indexMetadata.mapping()).thenReturn(mappingMetadata);
66+
67+
return new FieldStorageResolver(indexMetadata);
68+
}
69+
}

sandbox/plugins/analytics-engine/src/test/java/org/opensearch/analytics/planner/FilterRuleTests.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,9 @@ public void testFullTextErrorsWithoutDelegation() {
172172
RexNode condition = makeFullTextCall(FilterOperator.MATCH_PHRASE.toSqlFunction(), 0, "hello world");
173173
LogicalFilter filter = LogicalFilter.create(stubScan(table), condition);
174174

175-
PlannerContext context = buildContext("parquet", Map.of("message", Map.of("type", "keyword")));
175+
// index=false strips the inverted index so no backend can satisfy the full-text predicate
176+
// natively, forcing the "without delegation" code path under test.
177+
PlannerContext context = buildContext("parquet", Map.of("message", Map.of("type", "keyword", "index", false)));
176178

177179
IllegalStateException exception = expectThrows(IllegalStateException.class, () -> runPlanner(filter, context));
178180
assertTrue(exception.getMessage().contains("No backend can evaluate filter predicate"));

0 commit comments

Comments
 (0)