ES|QL: register TABLES_TYPES capability and fix CSV tests#109926
ES|QL: register TABLES_TYPES capability and fix CSV tests#109926elasticsearchmachine merged 8 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| caps.add(LOOKUP_COMMAND); | ||
| caps.add(TABLES_TYPES); |
There was a problem hiding this comment.
nit: I'd argue that we should actually remove LOOKUP_COMMAND, as TABLES_TYPES introduced a breaking change and should therefore be treated as a LOOKUP_COMMAND_V2.
If we do this, we should align the yaml tests as well - these currently require both tables_types AND lookup_command and would be skipped if we removed LOOKUP_COMMAND.
For our test suites, it should make no difference if we have both caps here or only TABLES_TYPES; however, having both capabilities is like a node was saying "I can handle both the old and the new lookup!", which is wrong and could lead to failures in CCQ-scenarios where the remote cluster is newer than the local cluster.
nik9000
left a comment
There was a problem hiding this comment.
Yeah, it does make sense to swap it to the new one. It's nice to have the flexibility to just do whatever we need to do with these capabilities.
|
Thanks for the feedback folks! |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
So that registration is automatic. Let's merge it after #109926 to make sure all the tests are good
Registering
TABLES_TYPEScapability, that was created (but not registered) in #109489Also using it as a condition for
lookup.csv-spectests.These tests failed badly in Serverless multi-cluster due to different output format in two different nodes.
Probably (if I got it right) the YAML tests that were using this capability were not running at all. This PR should re-enable them.