Conversation
alex-spies
left a comment
There was a problem hiding this comment.
Backport looks alright to me except for a few lost updates and some of #144210 spilling into it. Mostly, the PromQL-related stuff should be removed before merging.
| // In PromQL, queries never fail due to a field not being mapped, instead an empty result is returned. | ||
| if (plan instanceof PromqlCommand) { | ||
| return resolve(plan, false); | ||
| } |
There was a problem hiding this comment.
PromQL-related changes don't need backporting. #141532 only made it into 9.4. This should be removed again.
| var unresolved = collectUnresolved(plan); | ||
| if (unresolved.isEmpty()) { | ||
| LinkedHashMap<String, List<UnresolvedAttribute>> unresolvedByName = collectUnresolved(plan); | ||
| if (unresolvedByName.isEmpty()) { | ||
| return plan; |
There was a problem hiding this comment.
This is including changes that are being backported in #144210. Let's merge the other one first.
| if (plan instanceof PromqlCommand promqlCommand) { | ||
| // The expressions of the PromqlCommand itself are not relevant here. | ||
| // The promqlPlan is a separate tree and its children may contain UnresolvedAttribute expressions | ||
| promqlCommand.promqlPlan().forEachExpressionDown(UnresolvedAttribute.class, collectUnresolved); |
There was a problem hiding this comment.
More PromQL changes that shouldn't be backported.
FWIW, the backport #144210 keeps on ignoring PromQL here, so it could make things a little easier once it's in 9.3
| @@ -29,7 +30,7 @@ public interface PlanStreamOutput { | |||
| * @param field The EsField to serialize | |||
There was a problem hiding this comment.
nit: missing the description of the transportVersion param which is added in #143399.
There was a problem hiding this comment.
Eval still in the comment
There was a problem hiding this comment.
Eval still in the comment
| var leftDne2 = as(leftEvalEval.fields().get(3), Alias.class); | ||
| assertThat(leftDne2.name(), is("does_not_exist2")); | ||
| assertThat(as(leftDne2.child(), Literal.class).dataType(), is(DataType.NULL)); | ||
| assertThat(leftDne2.id(), is(leftProject_does_not_exist2.id())); | ||
|
|
||
| var leftRel = as(leftEvalEval.child(), EsRelation.class); | ||
| assertThat(leftRel.indexPattern(), is("test")); | ||
|
|
||
| // Right branch: Project + Eval many nulls, Subquery -> Filter -> Eval -> EsRelation[languages] |
There was a problem hiding this comment.
Update got lost, EVAL still mentioned
(late) Manual backport of #143399