ESQL: More specific index pattern in testMultipleBatchesWithLookupJoin #130006
Conversation
Do not use `from *` in the test, be more specifc - otherwise other tests can affect the output if they leave indices behind, affecting the column count.
alex-spies
added
>test
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
alex-spies
added
the
auto-backport
| @@ -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\"}}}"); | |||
There was a problem hiding this comment.
I wonder if we should go one step forward and rely on randomIdentifier() for the index name here?
There was a problem hiding this comment.
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.
| 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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Assertion deleted in the next push!
That doesn't really tell us much in this test, anyway.
…-test-more-specific
alex-spies
added
the
auto-merge-without-approval
|
Only failure is in an unrelated spec test on Serverless, which this PR can't affect. This is safe to merge. |
💔 Backport failed
You can use sqren/backport to manually backport by running |
elastic#130006) * Specific idx in testMultipleBatchesWithLookupJoin Do not use `from *` in the test, be more specifc - otherwise other tests can affect the output if they leave indices behind, affecting the column count. * Remove column count validation That doesn't really tell us much in this test, anyway.
#130006) (#130116) * Specific idx in testMultipleBatchesWithLookupJoin Do not use `from *` in the test, be more specifc - otherwise other tests can affect the output if they leave indices behind, affecting the column count. * Remove column count validation That doesn't really tell us much in this test, anyway.
Do not use
from *in the test, be more specifc - otherwise other tests can affect the output if they leave indices behind, affecting the column count.We encountered a test failure in release tests; I wasn't able to reproduce, but this change should avoid any potential flakiness due to other tests affecting the global state that we run this test in.