Skip to content

Commit 7d340e8

Browse files
committed
Fix dedup 500 on cross-index object/scalar mapping conflict (opensearch-project#5685)
On the Calcite path, `dedup` pushes down as a composite aggregation whose surviving row is fetched via top_hits and decoded from the nested _source. When a wildcard spans indices that disagree on whether a path is an object or a scalar, the merged mapping types the path as an object (STRUCT) while some documents store a scalar there. Decoding that scalar as a struct threw `java.sql.SQLException: ... class java.lang.String cannot be cast to class java.util.Map`, failing the whole query (100% on the reporting customer's `source=logs-pr172502-*`). Guard the STRUCT branch of OpenSearchExprValueFactory.parse to degrade a scalar-under-object value to null, completing the graceful handling opensearch-project#5618 added for the geo_point and scalar branches. The catch is scoped to ClassCastException because this branch is also the whole-document parse entry point and each nested object level is wrapped independently, so a broader catch would swallow legitimate errors raised deeper in the recursion. Adds a factory unit regression plus two CalcitePPLDedupIT cases (object-vs- scalar and the mirror scalar-vs-object, the latter guarding opensearch-project#5618). Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent 81997e6 commit 7d340e8

3 files changed

Lines changed: 111 additions & 1 deletion

File tree

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLDedupIT.java

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import java.io.IOException;
1414
import org.json.JSONObject;
1515
import org.junit.jupiter.api.Test;
16+
import org.opensearch.client.Request;
17+
import org.opensearch.sql.legacy.TestUtils;
1618
import org.opensearch.sql.ppl.PPLIntegTestCase;
1719
import org.opensearch.sql.util.RequiresCapability;
1820

@@ -27,6 +29,87 @@ public void init() throws Exception {
2729
loadIndex(Index.ACCOUNT);
2830
}
2931

32+
/**
33+
* Regression test for https://github.com/opensearch-project/sql/issues/5685.
34+
*
35+
* <p>When {@code dedup} pushes down as a composite aggregation, the surviving row is fetched via
36+
* {@code top_hits}, whose nested {@code _source} is parsed back into a struct. If a wildcard
37+
* spans indices that disagree on whether an object path is an object or a scalar, the merged
38+
* mapping types the path as an object while some documents store a scalar there — and parsing
39+
* that scalar as a struct threw {@code java.sql.SQLException: ... String cannot be cast to ...
40+
* Map}, failing the whole query (100% on Fidelity's {@code source=logs-pr172502-*}). The query
41+
* must instead succeed and dedup on the requested key.
42+
*/
43+
@Test
44+
public void testDedupOverConflictingObjectScalarMapping() throws IOException {
45+
// idx A (dynamic:false): only maps `host`; keeps resource.attributes.k8s.namespace as a scalar
46+
// in _source (unmapped). idx B maps that same path as an OBJECT with a keyword leaf `name`.
47+
// Only B maps `resource`, so the wildcard merge classifies resource.attributes.k8s.namespace
48+
// as STRUCT deterministically, while A's doc holds a scalar there.
49+
if (!TestUtils.isIndexExist(client(), "dd5685_a")) {
50+
TestUtils.createIndexByRestClient(
51+
client(),
52+
"dd5685_a",
53+
"{\"mappings\":{\"dynamic\":false,\"properties\":{\"host\":{\"type\":\"keyword\"}}}}");
54+
Request a1 = new Request("PUT", "/dd5685_a/_doc/1?refresh=true");
55+
a1.setJsonEntity(
56+
"{\"host\":\"h1\",\"resource\":{\"attributes\":{\"k8s\":{\"namespace\":\"prod\"}}}}");
57+
client().performRequest(a1);
58+
}
59+
if (!TestUtils.isIndexExist(client(), "dd5685_b")) {
60+
TestUtils.createIndexByRestClient(
61+
client(),
62+
"dd5685_b",
63+
"{\"mappings\":{\"properties\":{\"host\":{\"type\":\"keyword\"},\"resource\":"
64+
+ "{\"properties\":{\"attributes\":{\"properties\":{\"k8s\":{\"properties\":"
65+
+ "{\"namespace\":{\"properties\":{\"name\":{\"type\":\"keyword\"}}}}}}}}}}}}");
66+
Request b1 = new Request("PUT", "/dd5685_b/_doc/1?refresh=true");
67+
b1.setJsonEntity(
68+
"{\"host\":\"h2\",\"resource\":{\"attributes\":{\"k8s\":{\"namespace\":{\"name\":\"pod-x\"}}}}}");
69+
client().performRequest(b1);
70+
}
71+
JSONObject actual = executeQuery("source=dd5685_* | dedup host | fields host");
72+
verifyDataRows(actual, rows("h1"), rows("h2"));
73+
}
74+
75+
/**
76+
* Mirror of {@link #testDedupOverConflictingObjectScalarMapping}: the reverse conflict, where the
77+
* merged mapping types a path as a scalar but some document stores an object there. Parsing that
78+
* object under a scalar type must not throw {@code ... Map cannot be cast to ... String}.
79+
* (Guarded upstream by the scalar-branch handling in #5618; kept here as an end-to-end
80+
* regression.)
81+
*/
82+
@Test
83+
public void testDedupOverConflictingScalarObjectMapping() throws IOException {
84+
// idx A maps resource.attributes.k8s.namespace as a scalar keyword; idx B (dynamic:false) does
85+
// not map it but stores an OBJECT there in _source. Only A maps the path, so the merged type
86+
// is keyword while B's doc holds an object under it.
87+
if (!TestUtils.isIndexExist(client(), "dd5685c_a")) {
88+
TestUtils.createIndexByRestClient(
89+
client(),
90+
"dd5685c_a",
91+
"{\"mappings\":{\"properties\":{\"host\":{\"type\":\"keyword\"},\"resource\":"
92+
+ "{\"properties\":{\"attributes\":{\"properties\":{\"k8s\":{\"properties\":"
93+
+ "{\"namespace\":{\"type\":\"keyword\"}}}}}}}}}}");
94+
Request a1 = new Request("PUT", "/dd5685c_a/_doc/1?refresh=true");
95+
a1.setJsonEntity(
96+
"{\"host\":\"h1\",\"resource\":{\"attributes\":{\"k8s\":{\"namespace\":\"prod\"}}}}");
97+
client().performRequest(a1);
98+
}
99+
if (!TestUtils.isIndexExist(client(), "dd5685c_b")) {
100+
TestUtils.createIndexByRestClient(
101+
client(),
102+
"dd5685c_b",
103+
"{\"mappings\":{\"dynamic\":false,\"properties\":{\"host\":{\"type\":\"keyword\"}}}}");
104+
Request b1 = new Request("PUT", "/dd5685c_b/_doc/1?refresh=true");
105+
b1.setJsonEntity(
106+
"{\"host\":\"h2\",\"resource\":{\"attributes\":{\"k8s\":{\"namespace\":{\"name\":\"pod-x\"}}}}}");
107+
client().performRequest(b1);
108+
}
109+
JSONObject actual = executeQuery("source=dd5685c_* | dedup host | fields host");
110+
verifyDataRows(actual, rows("h1"), rows("h2"));
111+
}
112+
30113
@Test
31114
public void testDedup() throws IOException {
32115
JSONObject actual =

opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,20 @@ private ExprValue parse(
222222
return parseArray(content, field, type, supportArrays);
223223
} else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Object))
224224
|| type == STRUCT) {
225-
return parseStruct(content, field, supportArrays);
225+
// A field mapped as an object can still hold a scalar value in a particular document — most
226+
// often when a wildcard/alias spans indices that disagree on whether a path is an object or a
227+
// scalar, so the merged mapping types it as an object (STRUCT) while some documents store a
228+
// scalar there. Parsing that scalar as a struct throws ClassCastException (String cannot be
229+
// cast to Map); return null for that field instead of failing the whole request, matching the
230+
// graceful handling already applied to the geo_point and scalar branches. See #5685. The
231+
// catch is scoped to ClassCastException (not Exception) because this branch is also the entry
232+
// point for whole-document parsing, and each nested object level is wrapped independently — a
233+
// broader catch would swallow legitimate errors raised deeper in the recursion.
234+
try {
235+
return parseStruct(content, field, supportArrays);
236+
} catch (ClassCastException e) {
237+
return ExprNullValue.of();
238+
}
226239
} else if (typeActionMap.containsKey(type)) {
227240
if (content.isArray()) {
228241
return parseArray(content, field, type, supportArrays);

opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,20 @@ public void constructStruct() {
728728
constructFromObject("structV", ImmutableMap.of("id", 1, "state", "WA")));
729729
}
730730

731+
/**
732+
* A field mapped as an object (STRUCT) can still carry a scalar value in a particular document —
733+
* e.g. when a wildcard/alias spans indices that disagree on whether the path is an object or a
734+
* scalar, so the merged mapping types it as an object while a document (in this aggregation /
735+
* {@code top_hits} decode path, an {@code ObjectContent}) stores a scalar. Parsing that scalar as
736+
* a struct must not throw {@code String cannot be cast to Map}; it degrades to null, matching the
737+
* graceful handling of the scalar/geo_point branches. Regression test for #5685.
738+
*/
739+
@Test
740+
public void constructStructWithScalarValueReturnsNull() {
741+
assertEquals(nullValue(), constructFromObject("structV", "prod"));
742+
assertEquals(nullValue(), constructFromObject("structV", 1));
743+
}
744+
731745
@Test
732746
public void constructIP() {
733747
final String ipString = "192.168.0.1";

0 commit comments

Comments
 (0)