imply-cheddar commented on code in PR #16707:
URL: https://github.com/apache/druid/pull/16707#discussion_r1695387271


##########
processing/src/main/java/org/apache/druid/frame/field/LongFieldReader.java:
##########
@@ -99,4 +107,135 @@ public boolean isNull()
       return super._isNull();
     }
   }
+
+  @Override
+  public Column makeRACColumn(Frame frame, RowSignature signature, String 
columnName)
+  {
+    return new LongFieldReaderColumn(frame, signature.indexOf(columnName), 
signature.size());
+  }
+
+  private class LongFieldReaderColumn implements Column
+  {
+    private final Frame frame;
+    private final Memory dataRegion;
+    private final FieldPositionHelper coach;
+
+    public LongFieldReaderColumn(Frame frame, int columnIndex, int numFields)
+    {
+      this.frame = frame;
+      this.dataRegion = frame.region(RowBasedFrameWriter.ROW_DATA_REGION);
+
+      this.coach = new FieldPositionHelper(
+          frame,
+          frame.region(RowBasedFrameWriter.ROW_OFFSET_REGION),
+          dataRegion,
+          columnIndex,
+          numFields
+      );
+    }
+
+    @Nonnull
+    @Override
+    public ColumnAccessor toAccessor()
+    {
+      return new ColumnAccessor()
+      {
+        @Override
+        public ColumnType getType()
+        {
+          return ColumnType.LONG;
+        }
+
+        @Override
+        public int numRows()
+        {
+          return frame.numRows();
+        }
+
+        @Override
+        public boolean isNull(int rowNum)
+        {
+          final long fieldPosition = coach.computeFieldPosition(rowNum);
+          return dataRegion.getByte(fieldPosition) == getNullIndicatorByte();
+        }
+
+        @Nullable
+        @Override
+        public Object getObject(int rowNum)
+        {
+          final long fieldPosition = coach.computeFieldPosition(rowNum);
+
+          if (dataRegion.getByte(fieldPosition) == getNullIndicatorByte()) {
+            return null;
+          } else {
+            return getLongAtPosition(fieldPosition);
+          }
+        }
+
+        @Override
+        public double getDouble(int rowNum)
+        {
+          return getLong(rowNum);
+        }
+
+        @Override
+        public float getFloat(int rowNum)
+        {
+          return getLong(rowNum);
+        }
+
+        @Override
+        public long getLong(int rowNum)
+        {
+          final long fieldPosition = coach.computeFieldPosition(rowNum);
+
+          if (dataRegion.getByte(fieldPosition) == getNullIndicatorByte()) {
+            return 0L;
+          } else {
+            return getLongAtPosition(fieldPosition);
+          }
+        }
+
+        @Override
+        public int getInt(int rowNum)
+        {
+          return (int) getLong(rowNum);
+        }
+
+        @Override
+        public int compareRows(int lhsRowNum, int rhsRowNum)
+        {
+          long lhsPosition = coach.computeFieldPosition(lhsRowNum);
+          long rhsPosition = coach.computeFieldPosition(rhsRowNum);
+
+          final byte nullIndicatorByte = getNullIndicatorByte();
+          if (dataRegion.getByte(lhsPosition) == nullIndicatorByte) {
+            if (dataRegion.getByte(rhsPosition) == nullIndicatorByte) {
+              return 0;
+            } else {
+              return -1;
+            }
+          } else {
+            if (dataRegion.getByte(rhsPosition) == nullIndicatorByte) {
+              return 1;
+            } else {
+              return Long.compare(getLongAtPosition(lhsPosition), 
getLongAtPosition(rhsPosition));
+            }
+          }
+        }

Review Comment:
   Which is the code to do that comparison with?  Or does it need to be 
written.  When I wrote this code, I know that I heard that the frames had this 
nice comparability property, but I couldn't figure out what code could be used 
to do it correctly, so the implementation fell to something that would 
definitely be correct.  Along with the statement of what should be done, a 
pointer to what code to use would make it much faster and simpler to actually 
update the PR and get it merged.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to