cryptoe commented on code in PR #15175:
URL: https://github.com/apache/druid/pull/15175#discussion_r1362500438
##########
processing/src/main/java/org/apache/druid/frame/field/RowMemoryFieldPointer.java:
##########
@@ -47,6 +61,56 @@ public RowMemoryFieldPointer(
@Override
public long position()
+ {
+ updatePosition();
+ return cachedPosition;
+ }
+
+ @Override
+ public long length()
+ {
+ updatePositionAndLength();
+ return cachedLength;
+ }
+
+ private void updatePosition()
+ {
+ long rowPointerPosition = rowPointer.position();
+ if (rowPointerPosition == rowWithPositionCached) {
+ return;
+ }
+ // Update the cached position for position()
+ rowWithPositionCached = rowPointerPosition;
+
+ // Update the start position
+ cachedPosition = startPosition(fieldNumber);
+ }
+
+ // Not all field types call length(), and therefore there's no need to cache
the length of the field
+ private void updatePositionAndLength()
+ {
+ updatePosition();
+
+ long rowPointerPosition = rowPointer.position();
Review Comment:
I think we should use `rowWithPositionCached` here
##########
processing/src/main/java/org/apache/druid/frame/field/RowMemoryFieldPointer.java:
##########
@@ -47,6 +61,56 @@ public RowMemoryFieldPointer(
@Override
public long position()
+ {
+ updatePosition();
+ return cachedPosition;
+ }
+
+ @Override
+ public long length()
+ {
+ updatePositionAndLength();
+ return cachedLength;
+ }
+
+ private void updatePosition()
+ {
+ long rowPointerPosition = rowPointer.position();
+ if (rowPointerPosition == rowWithPositionCached) {
Review Comment:
```suggestion
if (rowPointerPosition == rowWithCachedPosition) {
```
##########
processing/src/main/java/org/apache/druid/frame/field/RowMemoryFieldPointer.java:
##########
@@ -47,6 +61,56 @@ public RowMemoryFieldPointer(
@Override
public long position()
+ {
+ updatePosition();
+ return cachedPosition;
+ }
+
+ @Override
+ public long length()
+ {
+ updatePositionAndLength();
+ return cachedLength;
+ }
+
+ private void updatePosition()
+ {
+ long rowPointerPosition = rowPointer.position();
+ if (rowPointerPosition == rowWithPositionCached) {
+ return;
+ }
+ // Update the cached position for position()
+ rowWithPositionCached = rowPointerPosition;
+
+ // Update the start position
+ cachedPosition = startPosition(fieldNumber);
+ }
+
+ // Not all field types call length(), and therefore there's no need to cache
the length of the field
+ private void updatePositionAndLength()
+ {
+ updatePosition();
+
+ long rowPointerPosition = rowPointer.position();
+ if (rowPointerPosition == rowWithLengthCached) {
Review Comment:
```suggestion
if (rowPointerPosition == rowWithCachedLength) {
```
--
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]
