Skip to content
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1014,12 +1014,14 @@ public void testMultipleBatchesWithLookupJoin() throws IOException {
// Create more than 10 indices to trigger multiple batches of data node execution.
// The sort field should be missing on some indices to reproduce NullPointerException caused by duplicated items in layout
for (int i = 1; i <= 20; i++) {
createIndex("idx" + i, randomBoolean(), "\"mappings\": {\"properties\" : {\"a\" : {\"type\" : \"keyword\"}}}");
createIndex("no_sort_field_idx" + i, randomBoolean(), "\"mappings\": {\"properties\" : {\"a\" : {\"type\" : \"keyword\"}}}");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should go one step forward and rely on randomIdentifier() for the index name here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes clean up a little harder as I'd need to keep track of the index names to delete them later or make it so I can perform a wildcard delete. This should be fine, I don't think this was the root cause of the failure.

}
bulkLoadTestDataLookupMode(10);
// lookup join with and without sort
for (String sort : List.of("", "| sort integer")) {
var query = requestObjectBuilder().query(format(null, "from * | lookup join {} on integer {}", testIndexName(), sort));
var query = requestObjectBuilder().query(
format(null, "from {},no_sort_field_idx* | lookup join {} on integer {}", testIndexName(), testIndexName(), sort)
);
Map<String, Object> result = runEsql(query);
var columns = as(result.get("columns"), List.class);
assertEquals(22, columns.size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option is removing the column count validation, as this test needs to be updated every time a new field is added to the default index. It is weird where the extra column comes from though, I thought at the beginning of the tests, all existing indices are cleaned up, we should only have a deterministic number of columns in the resultset.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assertion deleted in the next push!

Expand All @@ -1028,7 +1030,7 @@ public void testMultipleBatchesWithLookupJoin() throws IOException {
}
// clean up
for (int i = 1; i <= 20; i++) {
assertThat(deleteIndex("idx" + i).isAcknowledged(), is(true));
assertThat(deleteIndex("no_sort_field_idx" + i).isAcknowledged(), is(true));
}
}

Expand Down