-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allowing non-rollup and rollup indices to be searched together #1268
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1092,12 +1092,12 @@ class RollupInterceptorIT : RollupRestTestCase() { | |
}, | ||
"aggs": { | ||
"sum_passenger_count": { "sum": { "field": "passenger_count" } }, | ||
"max_passenger_count": { "max": { "field": "passenger_count" } }, | ||
"value_count_passenger_count": { "value_count": { "field": "passenger_count" } } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why removing value_count? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was initially also not being used in the test earlier since we were testing the validation (where we used to throw the exception) on rollup and source index searches together |
||
"max_passenger_count": { "max": { "field": "passenger_count" } } | ||
} | ||
} | ||
""".trimIndent() | ||
// Search 1 non-rollup index and 1 rollup | ||
// Search 1 non-rollup index and 1 rollup | ||
updateSearchRawRollupClusterSetting(false) | ||
val searchResult1 = client().makeRequest("POST", "/$sourceIndex2,$targetIndex2/_search", emptyMap(), StringEntity(req, ContentType.APPLICATION_JSON)) | ||
assertTrue(searchResult1.restStatus() == RestStatus.OK) | ||
val failures = extractFailuresFromSearchResponse(searchResult1) | ||
|
@@ -1112,6 +1112,29 @@ class RollupInterceptorIT : RollupRestTestCase() { | |
"Not all indices have rollup job", failures?.get(0)?.get("reason") ?: "Didn't find failure reason in search response", | ||
) | ||
|
||
// Updating to allow searching on non-rollup and rolled-up index together | ||
updateSearchRawRollupClusterSetting(true) | ||
val rawRes1 = client().makeRequest("POST", "/$sourceIndex2/_search", emptyMap(), StringEntity(req, ContentType.APPLICATION_JSON)) | ||
assertTrue(rawRes1.restStatus() == RestStatus.OK) | ||
val rawRes2 = client().makeRequest("POST", "/$targetIndex2/_search", emptyMap(), StringEntity(req, ContentType.APPLICATION_JSON)) | ||
assertTrue(rawRes2.restStatus() == RestStatus.OK) | ||
val searchResult = client().makeRequest("POST", "/$sourceIndex2,$targetIndex2/_search", emptyMap(), StringEntity(req, ContentType.APPLICATION_JSON)) | ||
assertTrue(searchResult.restStatus() == RestStatus.OK) | ||
val rawAgg1Res = rawRes1.asMap()["aggregations"] as Map<String, Map<String, Any>> | ||
val rawAgg2Res = rawRes2.asMap()["aggregations"] as Map<String, Map<String, Any>> | ||
val rollupAggResMulti = searchResult.asMap()["aggregations"] as Map<String, Map<String, Any>> | ||
|
||
val trueAggSum = rawAgg1Res.getValue("sum_passenger_count")["value"] as Double + rawAgg2Res.getValue("sum_passenger_count")["value"] as Double | ||
|
||
assertEquals( | ||
"Searching single raw source index and rollup target index did not return the same sum results", | ||
rawAgg1Res.getValue("max_passenger_count")["value"], rollupAggResMulti.getValue("max_passenger_count")["value"], | ||
) | ||
assertEquals( | ||
"Searching rollup target index did not return the sum for all of the rollup jobs on the index", | ||
trueAggSum, rollupAggResMulti.getValue("sum_passenger_count")["value"], | ||
) | ||
|
||
// Search 2 rollups with different mappings | ||
try { | ||
client().makeRequest( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let move this to trace since multiple such search queries can lead to significant logging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done