Skip to content

Commit c87b5b8

Browse files
committed
fix tests
Signed-off-by: Jialiang Liang <jiallian@amazon.com>
1 parent 124c42a commit c87b5b8

4 files changed

Lines changed: 83 additions & 73 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2828,8 +2828,8 @@ public void testHighlightOsdObjectFormatExplain() throws IOException {
28282828
"{\"pre_tags\": [\"<b>\"], \"post_tags\": [\"</b>\"],"
28292829
+ " \"fields\": {\"*\": {}}, \"fragment_size\": 2147483647}";
28302830
var result = explainQueryYaml(query, highlightJson);
2831-
// Same explain plan as wildcard (fields: {"*": {}}) — just verify it doesn't error
2832-
String expected = loadExpectedPlan("explain_highlight_wildcard.yaml");
2831+
// OSD format includes pre_tags/post_tags in the highlight builder output
2832+
String expected = loadExpectedPlan("explain_highlight_osd_format.yaml");
28332833
assertYamlEqualsIgnoreId(expected, result);
28342834
}
28352835
}

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

Lines changed: 63 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import static org.opensearch.sql.plugin.rest.RestPPLQueryAction.QUERY_API_ENDPOINT;
1212

1313
import java.io.IOException;
14-
import java.util.Locale;
1514
import org.json.JSONArray;
1615
import org.json.JSONObject;
1716
import org.junit.Assert;
@@ -34,51 +33,56 @@ public void init() throws Exception {
3433

3534
@Test
3635
public void testHighlightWildcardWithSearchQuery() throws IOException {
37-
// Search for "Holmes" with wildcard highlight — _highlight should appear in results
3836
JSONObject result =
3937
executeQueryWithHighlight("source=" + TEST_INDEX_ACCOUNT + " \"Holmes\"", "[\"*\"]");
4038
JSONArray dataRows = result.getJSONArray("datarows");
4139
assertTrue(dataRows.length() > 0);
42-
assertSchemaContains(result, "_highlight");
40+
assertHighlightsExist(result);
4341
}
4442

4543
@Test
4644
public void testHighlightContainsMatchingFragments() throws IOException {
4745
JSONObject result =
4846
executeQueryWithHighlight("source=" + TEST_INDEX_ACCOUNT + " \"Holmes\"", "[\"*\"]");
49-
JSONArray dataRows = result.getJSONArray("datarows");
50-
assertTrue(dataRows.length() > 0);
51-
// Find the _highlight column index and verify it contains highlighted fragments
52-
int highlightIdx = getFieldIndex(result, "_highlight");
53-
assertTrue("_highlight column should exist", highlightIdx >= 0);
54-
// At least one row should have non-empty highlight data
55-
boolean foundHighlight = false;
56-
for (int i = 0; i < dataRows.length(); i++) {
57-
Object hlValue = dataRows.getJSONArray(i).get(highlightIdx);
58-
if (hlValue != null && !hlValue.equals(JSONObject.NULL)) {
59-
foundHighlight = true;
47+
JSONArray highlights = result.getJSONArray("highlights");
48+
assertTrue("highlights array should not be empty", highlights.length() > 0);
49+
// At least one highlight entry should have non-empty data
50+
boolean foundFragment = false;
51+
for (int i = 0; i < highlights.length(); i++) {
52+
JSONObject hlEntry = highlights.getJSONObject(i);
53+
if (hlEntry.length() > 0) {
54+
foundFragment = true;
6055
break;
6156
}
6257
}
63-
assertTrue("At least one row should have highlight data", foundHighlight);
58+
assertTrue("At least one highlight entry should have fragment data", foundFragment);
6459
}
6560

6661
@Test
6762
public void testHighlightOsdObjectFormat() throws IOException {
68-
// OSD sends highlight as a rich object with custom tags
6963
String highlightJson =
7064
"{\"pre_tags\": [\"<b>\"], \"post_tags\": [\"</b>\"],"
7165
+ " \"fields\": {\"*\": {}}, \"fragment_size\": 2147483647}";
7266
JSONObject result =
7367
executeQueryWithHighlight("source=" + TEST_INDEX_ACCOUNT + " \"Holmes\"", highlightJson);
7468
JSONArray dataRows = result.getJSONArray("datarows");
7569
assertTrue(dataRows.length() > 0);
76-
assertSchemaContains(result, "_highlight");
70+
assertHighlightsExist(result);
71+
// Verify custom tags are applied
72+
JSONArray highlights = result.getJSONArray("highlights");
73+
boolean foundCustomTag = false;
74+
for (int i = 0; i < highlights.length(); i++) {
75+
String hlStr = highlights.getJSONObject(i).toString();
76+
if (hlStr.contains("<b>")) {
77+
foundCustomTag = true;
78+
break;
79+
}
80+
}
81+
assertTrue("Highlights should use custom <b> tags", foundCustomTag);
7782
}
7883

7984
@Test
8085
public void testHighlightOsdObjectFormatWithDashboardsTags() throws IOException {
81-
// The exact format OSD uses with @opensearch-dashboards-highlighted-field@ tags
8286
String highlightJson =
8387
"{\"pre_tags\": [\"@opensearch-dashboards-highlighted-field@\"],"
8488
+ " \"post_tags\": [\"@/opensearch-dashboards-highlighted-field@\"],"
@@ -87,52 +91,59 @@ public void testHighlightOsdObjectFormatWithDashboardsTags() throws IOException
8791
executeQueryWithHighlight("source=" + TEST_INDEX_ACCOUNT + " \"Holmes\"", highlightJson);
8892
JSONArray dataRows = result.getJSONArray("datarows");
8993
assertTrue(dataRows.length() > 0);
90-
assertSchemaContains(result, "_highlight");
94+
assertHighlightsExist(result);
95+
// Verify dashboards tags are applied
96+
JSONArray highlights = result.getJSONArray("highlights");
97+
boolean foundDashboardsTag = false;
98+
for (int i = 0; i < highlights.length(); i++) {
99+
String hlStr = highlights.getJSONObject(i).toString();
100+
if (hlStr.contains("@opensearch-dashboards-highlighted-field@")) {
101+
foundDashboardsTag = true;
102+
break;
103+
}
104+
}
105+
assertTrue("Highlights should use OSD dashboards tags", foundDashboardsTag);
91106
}
92107

93108
@Test
94109
public void testHighlightWithFilter() throws IOException {
95110
JSONObject result =
96111
executeQueryWithHighlight(
97112
"source=" + TEST_INDEX_ACCOUNT + " \"Holmes\" | where age > 30", "[\"*\"]");
98-
assertSchemaContains(result, "_highlight");
113+
assertHighlightsExist(result);
99114
}
100115

101116
@Test
102117
public void testHighlightWithFields() throws IOException {
103118
JSONObject result =
104119
executeQueryWithHighlight(
105120
"source=" + TEST_INDEX_ACCOUNT + " \"Holmes\" | fields firstname, lastname", "[\"*\"]");
106-
assertSchemaContains(result, "_highlight");
121+
assertHighlightsExist(result);
107122
}
108123

109124
@Test
110125
public void testHighlightWithFetchSize() throws IOException {
111-
// Highlight combined with fetch_size
112126
Request request = new Request("POST", QUERY_API_ENDPOINT);
113-
String jsonBody =
114-
String.format(
115-
Locale.ROOT,
116-
"{\n \"query\": \"source=%s \\\"Holmes\\\"\",\n"
117-
+ " \"fetch_size\": 5,\n"
118-
+ " \"highlight\": [\"*\"]\n}",
119-
TEST_INDEX_ACCOUNT);
120-
request.setJsonEntity(jsonBody);
127+
JSONObject body = new JSONObject();
128+
body.put("query", "source=" + TEST_INDEX_ACCOUNT + " \"Holmes\"");
129+
body.put("fetch_size", 5);
130+
body.put("highlight", new JSONArray("[\"*\"]"));
131+
request.setJsonEntity(body.toString());
121132
RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
122133
restOptionsBuilder.addHeader("Content-Type", "application/json");
123134
request.setOptions(restOptionsBuilder);
124135
Response response = client().performRequest(request);
125136
Assert.assertEquals(200, response.getStatusLine().getStatusCode());
126137
JSONObject result = jsonify(getResponseBody(response, true));
127-
assertSchemaContains(result, "_highlight");
138+
assertHighlightsExist(result);
128139
}
129140

130141
@Test
131142
public void testHighlightWithSort() throws IOException {
132143
JSONObject result =
133144
executeQueryWithHighlight(
134145
"source=" + TEST_INDEX_ACCOUNT + " \"Holmes\" | sort age", "[\"*\"]");
135-
assertSchemaContains(result, "_highlight");
146+
assertHighlightsExist(result);
136147
}
137148

138149
@Test
@@ -143,21 +154,21 @@ public void testHighlightWithEval() throws IOException {
143154
+ TEST_INDEX_ACCOUNT
144155
+ " \"Holmes\" | eval age_plus_10 = age + 10 | fields firstname, age_plus_10",
145156
"[\"*\"]");
146-
assertSchemaContains(result, "_highlight");
157+
assertHighlightsExist(result);
147158
}
148159

149160
@Test
150161
public void testHighlightNoSearchQuery() throws IOException {
151-
// Without a search query, _highlight column appears but fragments may be empty
162+
// Without a search query, request should still succeed (highlights may or may not be present)
152163
JSONObject result = executeQueryWithHighlight("source=" + TEST_INDEX_BANK, "[\"*\"]");
153-
assertSchemaContains(result, "_highlight");
164+
assertTrue("Response should contain datarows", result.has("datarows"));
154165
}
155166

156167
@Test
157-
public void testWithoutHighlightNoHighlightColumn() throws IOException {
158-
// Without highlight parameter, _highlight should NOT appear in schema
168+
public void testWithoutHighlightNoHighlightArray() throws IOException {
169+
// Without highlight parameter, highlights array should NOT appear
159170
JSONObject result = executeQuery("source=" + TEST_INDEX_BANK);
160-
assertSchemaDoesNotContain(result, "_highlight");
171+
assertFalse("Response should NOT contain highlights array", result.has("highlights"));
161172
}
162173

163174
/**
@@ -170,9 +181,15 @@ public void testWithoutHighlightNoHighlightColumn() throws IOException {
170181
protected JSONObject executeQueryWithHighlight(String query, String highlightJson)
171182
throws IOException {
172183
Request request = new Request("POST", QUERY_API_ENDPOINT);
173-
request.setJsonEntity(
174-
String.format(
175-
Locale.ROOT, "{\n \"query\": \"%s\",\n \"highlight\": %s\n}", query, highlightJson));
184+
JSONObject body = new JSONObject();
185+
body.put("query", query);
186+
// Parse highlightJson to proper JSON type (array or object) so it serializes correctly
187+
Object highlightValue =
188+
highlightJson.trim().startsWith("[")
189+
? new JSONArray(highlightJson)
190+
: new JSONObject(highlightJson);
191+
body.put("highlight", highlightValue);
192+
request.setJsonEntity(body.toString());
176193
RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
177194
restOptionsBuilder.addHeader("Content-Type", "application/json");
178195
request.setOptions(restOptionsBuilder);
@@ -182,35 +199,10 @@ protected JSONObject executeQueryWithHighlight(String query, String highlightJso
182199
return jsonify(getResponseBody(response, true));
183200
}
184201

185-
/** Assert that the response schema contains a field with the given name. */
186-
private void assertSchemaContains(JSONObject result, String fieldName) {
187-
JSONArray schema = result.getJSONArray("schema");
188-
for (int i = 0; i < schema.length(); i++) {
189-
if (schema.getJSONObject(i).getString("name").equals(fieldName)) {
190-
return;
191-
}
192-
}
193-
Assert.fail("Schema should contain field: " + fieldName);
194-
}
195-
196-
/** Assert that the response schema does NOT contain a field with the given name. */
197-
private void assertSchemaDoesNotContain(JSONObject result, String fieldName) {
198-
JSONArray schema = result.getJSONArray("schema");
199-
for (int i = 0; i < schema.length(); i++) {
200-
if (schema.getJSONObject(i).getString("name").equals(fieldName)) {
201-
Assert.fail("Schema should NOT contain field: " + fieldName);
202-
}
203-
}
204-
}
205-
206-
/** Get the column index for a field name from the schema. */
207-
private int getFieldIndex(JSONObject result, String fieldName) {
208-
JSONArray schema = result.getJSONArray("schema");
209-
for (int i = 0; i < schema.length(); i++) {
210-
if (schema.getJSONObject(i).getString("name").equals(fieldName)) {
211-
return i;
212-
}
213-
}
214-
return -1;
202+
/** Assert that the response contains a non-empty highlights array. */
203+
private void assertHighlightsExist(JSONObject result) {
204+
assertTrue("Response should contain highlights array", result.has("highlights"));
205+
JSONArray highlights = result.getJSONArray("highlights");
206+
assertTrue("Highlights array should not be empty", highlights.length() > 0);
215207
}
216208
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
calcite:
2+
logical: |
3+
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
4+
LogicalProject(account_number=[$0], firstname=[$1], address=[$2], balance=[$3], gender=[$4], city=[$5], employer=[$6], state=[$7], age=[$8], email=[$9], lastname=[$10], _highlight=[$17])
5+
LogicalHighlight(highlightConfig=[[*]])
6+
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])
7+
physical: |
8+
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[HIGHLIGHT->[*], PROJECT->[account_number, firstname, address, balance, gender, city, employer, state, age, email, lastname, _highlight], LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","_source":{"includes":["account_number","firstname","address","balance","gender","city","employer","state","age","email","lastname"],"excludes":[]},"highlight":{"pre_tags":["<b>"],"post_tags":["</b>"],"fragment_size":2147483647,"fields":{"*":{}}}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
calcite:
2+
logical: |
3+
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
4+
LogicalProject(account_number=[$0], firstname=[$1], address=[$2], balance=[$3], gender=[$4], city=[$5], employer=[$6], state=[$7], age=[$8], email=[$9], lastname=[$10], _highlight=[$17])
5+
LogicalHighlight(highlightConfig=[[*]])
6+
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])
7+
physical: |
8+
EnumerableLimit(fetch=[10000])
9+
EnumerableCalc(expr#0..17=[{inputs}], proj#0..10=[{exprs}], _highlight=[$t17])
10+
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[HIGHLIGHT->[*]], OpenSearchRequestBuilder(sourceBuilder={"from":0,"timeout":"1m","highlight":{"pre_tags":["<b>"],"post_tags":["</b>"],"fragment_size":2147483647,"fields":{"*":{}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])

0 commit comments

Comments
 (0)