Fix first bug, lol

Author: alex-spies

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownJoinPastProject.java

@@ -10,6 +10,7 @@
 import org.elasticsearch.xpack.esql.core.expression.Alias;
 import org.elasticsearch.xpack.esql.core.expression.Attribute;
 import org.elasticsearch.xpack.esql.core.expression.AttributeMap;
+import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
 import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
@@ -64,46 +65,42 @@ protected LogicalPlan rule(Join join) {
             // used in the original Project; any such conflict needs to be resolved by copying the attribute under a temporary name via an
             // Eval - and using the attribute from said Eval in the new downstream Project.
             Set<String> lookupFieldNames = new HashSet<>(Expressions.names(join.rightOutputFields()));
-            PushDownUtils.AttributeReplacement replacement = PushDownUtils.renameAttributesInExpressions(lookupFieldNames, newProjections);
+            List<NamedExpression> finalProjections = new ArrayList<>(newProjections.size());
+            AttributeMap.Builder<Alias> aliasesForReplacedAttributesBuilder = AttributeMap.builder();
+            AttributeSet leftOutput = updatedJoin.left().outputSet();
 
-            List<Expression> conflictFreeProjections = replacement.rewrittenExpressions();
-            List<Alias> evalAliases = new ArrayList<>(replacement.replacedAttributes().values());
-
-            if (evalAliases.isEmpty()) {
-                // No name conflicts, so no eval needed.
-                return new Project(project.source(), updatedJoin.replaceLeft(project.child()), newProjections);
-            }
+            for (NamedExpression proj : newProjections) {
+                // TODO: add assert to Project that ensures Alias to attr or pure attr.
+                Attribute coreAttr = (Attribute) (proj instanceof Alias as ? as.child() : proj);
+                // Only fields from the left need to be protected from conflicts - because fields from the right shadow them.
+                if (leftOutput.contains(coreAttr) == false || lookupFieldNames.contains(coreAttr.name()) == false) {
+                    finalProjections.add(proj);
+                } else {
+                    // Conflict - the core attribute will be shadowed by the `LOOKUP JOIN` and we need to alias it in an upstream Eval.
+                    Alias renaming = aliasesForReplacedAttributesBuilder.computeIfAbsent(coreAttr, a -> {
+                        String tempName = TemporaryNameUtils.locallyUniqueTemporaryName(a.name(), "temp_name");
+                        return new Alias(a.source(), tempName, a, null, true);
+                    });
 
-            // The conflict free projections replaced any shadowed attributes by temporary attributes that we'll create in an Eval.
-            // That's good if the replaced attribute was in an Alias; if the projection was a mere attribute to begin with, we need to
-            // alias it back to the name/id that's expected in the original output.
-            List<NamedExpression> finalProjections = new ArrayList<>(conflictFreeProjections.size());
-            for (int i = 0; i < newProjections.size(); i++) {
-                Expression conflictFreeProj = conflictFreeProjections.get(i);
-                if (conflictFreeProj instanceof Alias as) {
-                    // Already aliased - keep it, it's fine if the child was rewritten.
-                    finalProjections.add(as);
-                } else if (conflictFreeProj instanceof Attribute conflictFreeAttr) {
-                    Attribute expectedOutputAttr = (Attribute) newProjections.get(i);
-                    if (expectedOutputAttr.semanticEquals(conflictFreeAttr)) {
-                        // no conflict, wasn't rewritten
-                        finalProjections.add(expectedOutputAttr);
+                    Attribute renamedAttribute = renaming.toAttribute();
+                    Alias renamedBack;
+                    if (proj instanceof Alias as) {
+                        renamedBack = new Alias(as.source(), as.name(), renamedAttribute, as.id(), as.synthetic());
                     } else {
-                        Alias renameBack = new Alias(
-                            expectedOutputAttr.source(),
-                            expectedOutputAttr.name(),
-                            conflictFreeAttr,
-                            expectedOutputAttr.id(),
-                            expectedOutputAttr.synthetic()
-                        );
-                        finalProjections.add(renameBack);
+                        // no alias - that means proj == coreAttr
+                        renamedBack = new Alias(coreAttr.source(), coreAttr.name(), renamedAttribute, coreAttr.id(), coreAttr.synthetic());
                     }
-                } else {
-                    throw new IllegalStateException("Resolving a list of projections must yield a list of projections again");
+                    finalProjections.add(renamedBack);
                 }
             }
 
-            Eval eval = new Eval(project.source(), project.child(), evalAliases);
+            if (aliasesForReplacedAttributesBuilder.isEmpty()) {
+                // No name conflicts, so no eval needed.
+                return new Project(project.source(), updatedJoin.replaceLeft(project.child()), newProjections);
+            }
+
+            List<Alias> renamesForEval = new ArrayList<>(aliasesForReplacedAttributesBuilder.build().values());
+            Eval eval = new Eval(project.source(), project.child(), renamesForEval);
             Join finalJoin = new Join(join.source(), eval, updatedJoin.right(), updatedJoin.config());
 
             return new Project(project.source(), finalJoin, finalProjections);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java

@@ -6850,7 +6850,15 @@ public void testMultipleLookupShadowing() {
     /**
      * Expects
      *
-     * TODO
+     * Project[[languages{f}#16, emp_no{f}#13, languages{f}#16 AS language_code#6, language_name{f}#27]]
+     * \_Limit[1000[INTEGER],true]
+     *   \_Join[LEFT,[languages{f}#16],[languages{f}#16],[language_code{f}#26]]
+     *     |_Limit[1000[INTEGER],true]
+     *     | \_Join[LEFT,[languages{f}#16],[languages{f}#16],[language_code{f}#24]]
+     *     |   |_Limit[1000[INTEGER],false]
+     *     |   | \_EsRelation[test][_meta_field{f}#19, emp_no{f}#13, first_name{f}#14, ..]
+     *     |   \_EsRelation[languages_lookup][LOOKUP][language_code{f}#24]
+     *     \_EsRelation[languages_lookup][LOOKUP][language_code{f}#26, language_name{f}#27]
      */
     public void testMultipleLookupProject() {
         // TODO a test case where pushing down past the RENAME would shadow