Add integer key comparator tests

lmdbjava · at055612 · Mar 6, 2025 · Mar 6, 2025 · Mar 6, 2025 · Mar 6, 2025
commit e2be6bf145a100fb2b8d40446dd3b70a54f51e2d
diff --git a/src/main/java/org/lmdbjava/AbstractFlagSet.java b/src/main/java/org/lmdbjava/AbstractFlagSet.java
@@ -26,8 +26,6 @@
 
 /**
  * Encapsulates an immutable set of flags and the associated bit mask for the flags in the set.
- *
- * @param <T>
  */
 public abstract class AbstractFlagSet<T extends Enum<T> & MaskedFlag> implements FlagSet<T> {
 

diff --git a/src/main/java/org/lmdbjava/ByteBufProxy.java b/src/main/java/org/lmdbjava/ByteBufProxy.java
@@ -23,6 +23,7 @@
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.PooledByteBufAllocator;
 import java.lang.reflect.Field;
+import java.nio.ByteOrder;
 import java.util.Comparator;
 import jnr.ffi.Pointer;
 
@@ -44,13 +45,6 @@ public final class ByteBufProxy extends BufferProxy<ByteBuf> {
   private static final String FIELD_NAME_ADDRESS = "memoryAddress";
   private static final String FIELD_NAME_LENGTH = "length";
   private static final String NAME = "io.netty.buffer.PooledUnsafeDirectByteBuf";
-  private static final Comparator<ByteBuf> comparator =
-      (o1, o2) -> {
-        requireNonNull(o1);
-        requireNonNull(o2);
-
-        return o1.compareTo(o2);
-      };
   private final long lengthOffset;
   private final long addressOffset;
 
@@ -81,6 +75,66 @@ public ByteBufProxy(final PooledByteBufAllocator allocator) {
     }
   }
 
+  /**
+   * Lexicographically compare two buffers.
+   *
+   * @param o1 left operand (required)
+   * @param o2 right operand (required)
+   * @return as specified by {@link Comparable} interface
+   */
+  public static int compareLexicographically(final ByteBuf o1, final ByteBuf o2) {
+    requireNonNull(o1);
+    requireNonNull(o2);
+    return o1.compareTo(o2);
+  }
+
+  /**
+   * Buffer comparator specifically for 4/8 byte keys that are unsigned ints/longs,
+   * i.e. when using MDB_INTEGER_KEY/MDB_INTEGERDUP. Compares the buffers numerically.
+   *
+   * @param o1 left operand (required)
+   * @param o2 right operand (required)
+   * @return as specified by {@link Comparable} interface
+   */
+  public static int compareAsIntegerKeys(final ByteBuf o1, final ByteBuf o2) {
+    requireNonNull(o1);
+    requireNonNull(o2);
+    // Both buffers should be same length according to LMDB API.
+    // From the LMDB docs for MDB_INTEGER_KEY
+    // numeric keys in native byte order: either unsigned int or size_t. The keys must all be of the same size.
+    final int len1 = o1.readableBytes();
+    final int len2 = o2.readableBytes();
+    if (len1 != len2) {
+      throw new RuntimeException("Length mismatch, len1: " + len1 + ", len2: " + len2
+          + ". Lengths must be identical and either 4 or 8 bytes.");
+    }
+    if (len1 == 8) {
+      final long lw;
+      final long rw;
+      if (ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN) {
+        lw = o1.readLongLE();
+        rw = o2.readLongLE();
+      } else {
+        lw = o1.readLong();
+        rw = o2.readLong();
+      }
+      return Long.compareUnsigned(lw, rw);
+    } else if (len1 == 4) {
+      final int lw;
+      final int rw;
+      if (ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN) {
+        lw = o1.readIntLE();
+        rw = o2.readIntLE();
+      } else {
+        lw = o1.readInt();
+        rw = o2.readInt();
+      }
+      return Integer.compareUnsigned(lw, rw);
+    } else {
+      return compareLexicographically(o1, o2);
+    }
+  }
+
   static Field findField(final String c, final String name) {
     Class<?> clazz;
     try {
@@ -115,7 +169,11 @@ protected ByteBuf allocate() {
 
   @Override
   public Comparator<ByteBuf> getComparator(final DbiFlagSet dbiFlagSet) {
-    return comparator;
+    if (dbiFlagSet.areAnySet(DbiFlagSet.INTEGER_KEY_FLAGS)) {
+      return ByteBufProxy::compareAsIntegerKeys;
+    } else {
+      return ByteBufProxy::compareLexicographically;
+    }
   }
 
   @Override

diff --git a/src/main/java/org/lmdbjava/ByteBufferProxy.java b/src/main/java/org/lmdbjava/ByteBufferProxy.java
@@ -60,6 +60,7 @@ public final class ByteBufferProxy {
    */
   public static final BufferProxy<ByteBuffer> PROXY_SAFE;
 
+  private static final ByteOrder NATIVE_ORDER = ByteOrder.nativeOrder();
 
   static {
     PROXY_SAFE = new ReflectiveProxy();
@@ -172,8 +173,8 @@ public static int compareAsIntegerKeys(final ByteBuffer o1, final ByteBuffer o2)
             + ". Lengths must be identical and either 4 or 8 bytes.");
       }
       // Keys for MDB_INTEGER_KEY are written in native order so ensure we read them in that order
-      o1.order(ByteOrder.nativeOrder());
-      o2.order(ByteOrder.nativeOrder());
+      o1.order(NATIVE_ORDER);
+      o2.order(NATIVE_ORDER);
       // TODO it might be worth the DbiBuilder having a method to capture fixedKeyLength() or -1
       //  for variable length keys. This can be passed to getComparator(..) so it can return a
       //  comparator that doesn't need to test the length every time. There may be other benefits

diff --git a/src/main/java/org/lmdbjava/DbiBuilder.java b/src/main/java/org/lmdbjava/DbiBuilder.java
@@ -53,20 +53,20 @@
    *            (see also {@link DbiBuilder#withoutDbName()})
    * @return The next builder stage.
    */
-  public DbiBuilderStage2<T> withDbName(final String name) {
+  public DbiBuilderStage2<T> setDbName(final String name) {
     // Null name is allowed so no null check
     final byte[] nameBytes = name == null
         ? null
         : name.getBytes(Env.DEFAULT_NAME_CHARSET);
-    return withDbName(nameBytes);
+    return setDbName(nameBytes);
   }
 
   /**
    * Create the {@link Dbi} with the passed name in byte[] form.
    * @param name The name of the database in byte form.
    * @return The next builder stage.
    */
-  public DbiBuilderStage2<T> withDbName(final byte[] name) {
+  public DbiBuilderStage2<T> setDbName(final byte[] name) {
     // Null name is allowed so no null check
     this.name = name;
     return new DbiBuilderStage2<>(this);
@@ -78,15 +78,15 @@
    * </p>
    * <p>
    * Equivalent to passing null to
-   * {@link DbiBuilder#withDbName(String)} or {@link DbiBuilder#withDbName(byte[])}.
+   * {@link DbiBuilder#setDbName(String)} or {@link DbiBuilder#setDbName(byte[])}.
    * </p>
    * <p>Note: The 'unnamed database' is used by LMDB to store the names of named databases, with
    * the database name being the key. Use of the unnamed database is intended for simple applications
    * with only one database.</p>
    * @return The next builder stage.
    */
   public DbiBuilderStage2<T> withoutDbName() {
-    return withDbName((byte[]) null);
+    return setDbName((byte[]) null);
   }
 
 
@@ -254,16 +254,16 @@
      * </p>
      * <p>
      * Clears all flags currently set by previous calls to
-     * {@link DbiBuilderStage3#withDbiFlags(Collection)},
-     * {@link DbiBuilderStage3#withDbiFlags(DbiFlags...)}
+     * {@link DbiBuilderStage3#setDbiFlags(Collection)},
+     * {@link DbiBuilderStage3#setDbiFlags(DbiFlags...)}
      * or {@link DbiBuilderStage3#addDbiFlag(DbiFlags)}.
      * </p>
      *
      * @param dbiFlags to open the database with.
      *                 A null {@link Collection} will just clear all set flags.
      *                 Null items are ignored.
      */
-    public DbiBuilderStage3<T> withDbiFlags(final Collection<DbiFlags> dbiFlags) {
+    public DbiBuilderStage3<T> setDbiFlags(final Collection<DbiFlags> dbiFlags) {
       flagSetBuilder.clear();
       if (dbiFlags != null) {
         dbiFlags.stream()
@@ -279,16 +279,16 @@
      * </p>
      * <p>
      * Clears all flags currently set by previous calls to
-     * {@link DbiBuilderStage3#withDbiFlags(Collection)},
-     * {@link DbiBuilderStage3#withDbiFlags(DbiFlags...)}
+     * {@link DbiBuilderStage3#setDbiFlags(Collection)},
+     * {@link DbiBuilderStage3#setDbiFlags(DbiFlags...)}
      * or {@link DbiBuilderStage3#addDbiFlag(DbiFlags)}.
      * </p>
      *
      * @param dbiFlags to open the database with.
      *                 A null array will just clear all set flags.
      *                 Null items are ignored.
      */
-    public DbiBuilderStage3<T> withDbiFlags(final DbiFlags... dbiFlags) {
+    public DbiBuilderStage3<T> setDbiFlags(final DbiFlags... dbiFlags) {
       flagSetBuilder.clear();
       if (dbiFlags != null) {
         Arrays.stream(dbiFlags)
@@ -304,15 +304,15 @@
      * </p>
      * <p>
      * Clears all flags currently set by previous calls to
-     * {@link DbiBuilderStage3#withDbiFlags(Collection)},
-     * {@link DbiBuilderStage3#withDbiFlags(DbiFlags...)}
+     * {@link DbiBuilderStage3#setDbiFlags(Collection)},
+     * {@link DbiBuilderStage3#setDbiFlags(DbiFlags...)}
      * or {@link DbiBuilderStage3#addDbiFlag(DbiFlags)}.
      * </p>
      *
      * @param dbiFlagSet to open the database with.
      *                 A null value will just clear all set flags.
      */
-    public DbiBuilderStage3<T> withDbiFlags(final DbiFlagSet dbiFlagSet) {
+    public DbiBuilderStage3<T> setDbiFlags(final DbiFlagSet dbiFlagSet) {
       flagSetBuilder.clear();
       if (dbiFlagSet != null) {
         this.flagSetBuilder.withFlags(dbiFlagSet.getFlags());
@@ -322,8 +322,8 @@
 
     /**
      * Adds a dbiFlag to those flags already added to this builder by
-     * {@link DbiBuilderStage3#withDbiFlags(DbiFlags...)},
-     * {@link DbiBuilderStage3#withDbiFlags(Collection)}
+     * {@link DbiBuilderStage3#setDbiFlags(DbiFlags...)},
+     * {@link DbiBuilderStage3#setDbiFlags(Collection)}
      * or {@link DbiBuilderStage3#addDbiFlag(DbiFlags)}.
      *
      * @param dbiFlag to add to any existing flags. A null value is a no-op.
@@ -336,8 +336,8 @@
 
     /**
      * Adds a dbiFlag to those flags already added to this builder by
-     * {@link DbiBuilderStage3#withDbiFlags(DbiFlags...)},
-     * {@link DbiBuilderStage3#withDbiFlags(Collection)}
+     * {@link DbiBuilderStage3#setDbiFlags(DbiFlags...)},
+     * {@link DbiBuilderStage3#setDbiFlags(Collection)}
      * or {@link DbiBuilderStage3#addDbiFlag(DbiFlags)}.
      *
      * @param dbiFlagSet to add to any existing flags. A null value is a no-op.
@@ -366,8 +366,8 @@
     *            with the {@link EnvFlags#MDB_RDONLY_ENV} flag, the {@link Txn} can be read-only,
      *            else it needs to be a read/write {@link Txn}.
      * @return this builder instance.
      */
-    public DbiBuilderStage3<T> withTxn(final Txn<T> txn) {
+    public DbiBuilderStage3<T> setTxn(final Txn<T> txn) {
       this.txn = Objects.requireNonNull(txn);
       return this;
     }

diff --git a/src/main/java/org/lmdbjava/DirectBufferProxy.java b/src/main/java/org/lmdbjava/DirectBufferProxy.java
@@ -22,6 +22,7 @@
 import static org.lmdbjava.UnsafeAccess.UNSAFE;
 
 import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
 import java.util.ArrayDeque;
 import java.util.Comparator;
 import jnr.ffi.Pointer;
@@ -50,6 +51,8 @@ public final class DirectBufferProxy extends BufferProxy<DirectBuffer> {
   private static final ThreadLocal<ArrayDeque<DirectBuffer>> BUFFERS =
       withInitial(() -> new ArrayDeque<>(16));
 
+  private static final ByteOrder NATIVE_ORDER = ByteOrder.nativeOrder();
+
   private DirectBufferProxy() {}
 
   /**
@@ -110,16 +113,19 @@ public static int compareAsIntegerKeys(final DirectBuffer o1, final DirectBuffer
           + ". Lengths must be identical and either 4 or 8 bytes.");
     }
     if (len1 == 8) {
-      final long lw = o1.getLong(0, BIG_ENDIAN);
-      final long rw = o2.getLong(0, BIG_ENDIAN);
+      final long lw = o1.getLong(0, NATIVE_ORDER);
+      final long rw = o2.getLong(0, NATIVE_ORDER);
       return Long.compareUnsigned(lw, rw);
     } else if (len1 == 4) {
-      final int lw = o1.getInt(0, BIG_ENDIAN);
-      final int rw = o2.getInt(0, BIG_ENDIAN);
+      final int lw = o1.getInt(0, NATIVE_ORDER);
+      final int rw = o2.getInt(0, NATIVE_ORDER);
       return Integer.compareUnsigned(lw, rw);
     } else {
-      throw new RuntimeException("Unexpected length len1: " + len1 + ", len2: " + len2
-          + ". Lengths must be identical and either 4 or 8 bytes.");
+      // size_t and int are likely to be 8bytes and 4bytes respectively on 64bit.
+      // If 32bit then would be 4/2 respectively.
+      // Short.compareUnsigned is not available in Java8.
+      // For now just fall back to our standard comparator
+      return compareLexicographically(o1, o2);
     }
   }
-Original file line number
+Diff line change
@@ Expand Up / @@ -26,8 +26,6 @@ @@
     /**
      * Encapsulates an immutable set of flags and the associated bit mask for the flags in the set.
-     *
-     * @param <T>
      */
     public abstract class AbstractFlagSet<T extends Enum<T> & MaskedFlag> implements FlagSet<T> {
@@ Expand Down @@