[GitHub] HanumathRao commented on a change in pull request #1381: DRILL-6475: Unnest: Null fieldId Pointer.

[GitBox <gi...@apache.org>]

HanumathRao commented on a change in pull request #1381: DRILL-6475: Unnest: Null fieldId Pointer.
URL: https://github.com/apache/drill/pull/1381#discussion_r202772270
 
 

 ##########
 File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/JoinPrelRenameVisitor.java
 ##########
 @@ -19,36 +19,70 @@
 
 import java.util.ArrayList;
 import java.util.List;
-
+import java.util.Map;
+import java.util.HashMap;
+
+import com.google.common.base.Preconditions;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
 import org.apache.drill.exec.planner.physical.JoinPrel;
 import org.apache.drill.exec.planner.physical.LateralJoinPrel;
 import org.apache.drill.exec.planner.physical.Prel;
 import org.apache.calcite.rel.RelNode;
 
 import com.google.common.collect.Lists;
+import org.apache.drill.exec.planner.physical.UnnestPrel;
 
 public class JoinPrelRenameVisitor extends BasePrelVisitor<Prel, Void, RuntimeException>{
 
+  private final Map<String, Prel> sourceOperatorRegistry = new HashMap();
+
   private static JoinPrelRenameVisitor INSTANCE = new JoinPrelRenameVisitor();
 
   public static Prel insertRenameProject(Prel prel){
     return prel.accept(INSTANCE, null);
   }
 
+  private void register(Prel toRegister) {
+    this.sourceOperatorRegistry.put(toRegister.getClass().getSimpleName(), toRegister);
 
 Review comment:
   Yes, I agree that each lateral can be associated to many unnest. I think the current design itself doesn't take into account the correct association with unnest and lateral. It traverses the right of the lateral looking for unnest and associates them. This can cause potential issues when we support binary operators like union, join etc. I think ideally we should have an id with each lateral and then associate that id to an unnest. This id can be used in the map while traversal etc. But for now as we don't support binary operators I have used the classname instead of id's to associate the lateral and unnest. Let me know if you have a better way to fix this issue.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services