kadirozde commented on code in PR #1937:
URL: https://github.com/apache/phoenix/pull/1937#discussion_r1757955825


##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/RowKeySchema.java:
##########
@@ -232,11 +262,24 @@ public Boolean previous(ImmutableBytesWritable ptr, int 
position, int minOffset)
             byte[] buf = ptr.get();
             int offset = ptr.getOffset()-1-offsetAdjustment;
             // Separator always zero byte if zero length
-            if (offset > minOffset && buf[offset] != 
QueryConstants.SEPARATOR_BYTE) {
-                byte sepByte = 
SchemaUtil.getSeparatorByte(rowKeyOrderOptimizable, false, field);
-                do {
-                    offset--;
-                } while (offset > minOffset && buf[offset] != sepByte);
+            if (field.getDataType() != PVarbinaryEncoded.INSTANCE) {
+                if (offset > minOffset && buf[offset] != 
QueryConstants.SEPARATOR_BYTE) {
+                    byte sepByte =
+                        SchemaUtil.getSeparatorByte(rowKeyOrderOptimizable, 
false, field);
+                    do {
+                        offset--;
+                    } while (offset > minOffset && buf[offset] != sepByte);
+                }
+            } else {
+                if (offset > minOffset && 
SchemaUtil.areSeparatorBytesForVarBinaryEncoded(buf,
+                    offset, SortOrder.ASC)) {
+                    boolean ascSort =
+                        !rowKeyOrderOptimizable || field.getSortOrder() == 
SortOrder.ASC;
+                    do {
+                        offset++;

Review Comment:
   is this supposed to be offset-- ?



##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/RowKeySchema.java:
##########
@@ -159,30 +160,59 @@ private Boolean next(ImmutableBytesWritable ptr, int 
position, int maxOffset, bo
         // If positioned at SEPARATOR_BYTE, skip it.
         // Don't look back at previous fields if this is our first next call, 
as
         // we may have a partial key for RVCs that doesn't include the leading 
field.
-        if (position > 0 && !isFirst && 
!getField(position-1).getDataType().isFixedWidth()) {
-            ptr.set(ptr.get(), ptr.getOffset()+ptr.getLength()+1, 0);
+        if (position > 0 && !isFirst && !getField(position - 
1).getDataType().isFixedWidth()) {
+            if (getField(position - 1).getDataType() != 
PVarbinaryEncoded.INSTANCE) {

Review Comment:
   Can we replace hard coded values 1 and 2 with properly named constants?



-- 
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: issues-unsubscr...@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to