[GitHub] [calcite] rubenada commented on a diff in pull request #3311: [CALCITE-5732] EnumerableHashJoin and EnumerableMergeJoin on composite key return rows matching condition 'null = null'

Thu, 13 Jul 2023 01:53:19 -0700 


rubenada commented on code in PR #3311:
URL: https://github.com/apache/calcite/pull/3311#discussion_r1262238357


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/PhysType.java:
##########
@@ -117,6 +117,25 @@ Expression fieldReference(Expression expression, int field,
    */
   Expression generateAccessor(List<Integer> fields);
 
+  /** Similar to {@link #generateAccessor(List)}, but if one of the fields is 
<code>null</code>,
+   * it will return <code>null</code>.
+   *
+   * <p>For example:
+   *
+   * <blockquote><pre>
+   * new Function1&lt;Employee, Object[]&gt; {
+   *    public Object[] apply(Employee v1) {
+   *        return v1.&lt;fieldN&gt; == null
+   *            ? null
+   *            : v1.&lt;fieldM&gt; == null
+   *                ? null
+   *                : FlatLists.of(v1.&lt;fieldN&gt;, v1.&lt;fieldM&gt;);
+   *    }
+   * }
+   * }</pre></blockquote>
+   */
+  Expression generateNullableAccessor(List<Integer> fields);

Review Comment:
   This is a fair point. It could be done, but I think that the "old" method 
was still useful. Even though it would not be used any more within Calcite 
core, it might be used by some Calcite clients (and I didn't want to "break" 
their code by changing the implementation); so I considered it was arguably 
better (and probably safer) to keep the old method as it was, and add a new one 
with the peculiarities which are required for our hashJoin algorithm.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to