yashmayya commented on code in PR #16152:
URL: https://github.com/apache/pinot/pull/16152#discussion_r2156732373
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/operands/TransformOperand.java:
##########
@@ -28,4 +29,7 @@ public interface TransformOperand {
@Nullable
Object apply(Object[] row);
+
+ @Nullable
+ Object apply(List<Object> row);
Review Comment:
This is leading to a lot of code duplication across all implementations.
Could we implement the other `apply` method as a default method here in this
interface, delegating to this method using `Arrays.asList` (that shouldn't
result in any additional copying or allocation)?
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/BaseJoinOperator.java:
##########
@@ -376,4 +382,126 @@ public StatMap.Type getType() {
return _type;
}
}
+
+ /**
+ * This util class is a view over the left and right row joined together
+ * currently this is used for filtering and input of projection. So if the
joined
+ * tuple doesn't pass the predicate, the join result is not materialized
into Object[].
+ *
+ * It is debatable whether we always want to use this instead of copying the
tuple
+ */
+ private abstract static class JoinedRowView extends AbstractList<Object>
implements List<Object> {
Review Comment:
Would it be possible to enhance and re-use this existing util class instead
-
https://github.com/apache/pinot/blob/60f1897d95024db29b820fdd87c1962809f2e9fb/pinot-common/src/main/java/org/apache/pinot/common/utils/list/FlatViewList.java#L31
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/BaseJoinOperator.java:
##########
@@ -376,4 +382,126 @@ public StatMap.Type getType() {
return _type;
}
}
+
+ /**
+ * This util class is a view over the left and right row joined together
+ * currently this is used for filtering and input of projection. So if the
joined
+ * tuple doesn't pass the predicate, the join result is not materialized
into Object[].
+ *
+ * It is debatable whether we always want to use this instead of copying the
tuple
+ */
+ private abstract static class JoinedRowView extends AbstractList<Object>
implements List<Object> {
+ protected final int _leftSize;
+ protected final int _size;
+
+ protected JoinedRowView(int resultColumnSize, int leftSize) {
+ _leftSize = leftSize;
+ _size = resultColumnSize;
+ }
+
+ private static final class NullNullView extends JoinedRowView {
Review Comment:
When would both the sides be `null`?
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/BaseJoinOperator.java:
##########
@@ -376,4 +382,126 @@ public StatMap.Type getType() {
return _type;
}
}
+
+ /**
+ * This util class is a view over the left and right row joined together
+ * currently this is used for filtering and input of projection. So if the
joined
+ * tuple doesn't pass the predicate, the join result is not materialized
into Object[].
+ *
+ * It is debatable whether we always want to use this instead of copying the
tuple
+ */
+ private abstract static class JoinedRowView extends AbstractList<Object>
implements List<Object> {
+ protected final int _leftSize;
+ protected final int _size;
+
+ protected JoinedRowView(int resultColumnSize, int leftSize) {
+ _leftSize = leftSize;
+ _size = resultColumnSize;
+ }
+
+ private static final class NullNullView extends JoinedRowView {
+ public NullNullView(int resultColumnSize, int leftSize) {
+ super(resultColumnSize, leftSize);
+ }
+
+ @Override
+ public Object get(int i) {
+ return null;
+ }
+
+ @Override
+ @NotNull
+ public Object[] toArray() {
+ return new Object[_size];
+ }
+ }
+
+ private static final class LeftRightView extends JoinedRowView {
+ private final Object[] _leftRow;
+ private final Object[] _rightRow;
+
+ private LeftRightView(Object[] leftRow, Object[] rightRow, int
resultColumnSize, int leftSize) {
+ super(resultColumnSize, leftSize);
+ _leftRow = leftRow;
+ _rightRow = rightRow;
+ }
+
+ @Override
+ public Object get(int i) {
+ return i < _leftSize ? _leftRow[i] : _rightRow[i - _leftSize];
+ }
+
+ @Override
+ @NotNull
+ public Object[] toArray() {
+ Object[] resultRow = new Object[_size];
+ System.arraycopy(_leftRow, 0, resultRow, 0, _leftSize);
+ System.arraycopy(_rightRow, 0, resultRow, _leftSize, _rightRow.length);
+ return resultRow;
+ }
+ }
+
+ private static final class NullRightView extends JoinedRowView {
+ private final Object[] _rightRow;
+
+ public NullRightView(Object[] rightRow, int resultColumnSize, int
leftSize) {
+ super(resultColumnSize, leftSize);
+ _rightRow = rightRow;
+ }
+
+ @Override
+ public Object get(int i) {
+ return i < _leftSize ? null : _rightRow[i - _leftSize];
+ }
+
+ @Override
+ @NotNull
+ public Object[] toArray() {
+ Object[] resultRow = new Object[_size];
+ System.arraycopy(_rightRow, 0, resultRow, _leftSize, _rightRow.length);
+ return resultRow;
+ }
+ }
+
+ private static final class LeftNullView extends JoinedRowView {
Review Comment:
The name is a little ambiguous IMO and could be confusing (left row + null,
or is the left side null).
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/BaseJoinOperator.java:
##########
@@ -41,6 +42,7 @@
import org.apache.pinot.spi.utils.BooleanUtils;
import
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey;
import
org.apache.pinot.spi.utils.CommonConstants.MultiStageQueryRunner.JoinOverFlowMode;
+import org.codehaus.commons.nullanalysis.NotNull;
Review Comment:
We don't use this annotation anywhere else, let's not introduce it here.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
