From b9b857b0b1f3f9df35ea7ecf9e88e1e5550bd43c Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Wed, 12 Aug 2026 18:20:57 +0000
Subject: [PATCH] Serve Cassandra cursor repositioning with CQL slice queries instead of a no-op iterator restart (#865)

---
 opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java |  243 +++++++++++++++++++-----
 opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java   |  329 ++++++++++++++++++++++++++++++++
 2 files changed, 518 insertions(+), 54 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java
index 7efeb8c..e30c748 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java
@@ -58,6 +58,7 @@
 
 import com.datastax.oss.driver.api.core.CqlSession;
 import com.datastax.oss.driver.api.core.config.DriverConfigLoader;
+import com.datastax.oss.driver.api.core.cql.BoundStatement;
 import com.datastax.oss.driver.api.core.cql.PreparedStatement;
 import com.datastax.oss.driver.api.core.cql.ResultSet;
 import com.datastax.oss.driver.api.core.cql.Row;
@@ -71,7 +72,18 @@
 	private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
 
 	private CASBackendCfg config;
-	
+
+	//a cursor starts with a small page so that a point lookup does not transfer thousands of rows,
+	//and doubles it while a scan keeps outrunning it (the driver default page is 5000 rows)
+	final int initialPageSize=Math.max(1,Integer.getInteger("org.openidentityplatform.opendj.cassandra.fetchsize.initial",32));
+	final int maxPageSize=Math.max(initialPageSize,Integer.getInteger("org.openidentityplatform.opendj.cassandra.fetchsize",1000));
+
+	//CursorImpl.forwardInPage outcomes
+	final static int POSITIONED=1;
+	final static int MISSING=0;
+	final static int NEEDS_SEEK=-1;
+	final static int PAGE_OUT=-2;
+
 	public CASStorage(CASBackendCfg cfg, ServerContext serverContext) {
 		this.config = cfg;
 		cfg.addCASChangeListener(this);
@@ -292,42 +304,138 @@
 		}
 	}
 	
-	private final class CursorImpl implements Cursor<ByteString, ByteString> {
+	// Iterates the (baseDN,indexId) partition in key order.
+	// A ResultSet can only be consumed once: rc.iterator() always returns the same iterator,
+	// so "restarting" it never rewinds. Every repositioning that cannot be served by moving
+	// forward within the already-fetched rows therefore runs a new server-side slice query
+	// on the "key" clustering column instead.
+	// Pages are sized by the cursor rather than by the driver default of 5000 rows: a point
+	// lookup must not transfer thousands of entries, so a cursor starts with a small page and
+	// doubles it (up to maxPageSize) every time a scan outruns it. Scanning past the end of a
+	// page runs the next slice from the current key instead of letting the driver page with the
+	// size the cursor started with.
+	final class CursorImpl implements Cursor<ByteString, ByteString> {
 		final TreeName treeName;
 		final TransactionImpl tx;
+		final String tableName=getTableName();
+
+		//visible for tests
+		long queryCount;
+		int pageSize=initialPageSize;
 
 		ResultSet rc;
 		Iterator<Row> iterator;
+		//the row the cursor is on, or - when defined is false - the row a failed positionToKey
+		//stopped just before, which is where next() resumes (same as pdb)
 		Row current=null;
-		
+		boolean defined=false;
+		//rc holds a DESC page: it must never serve a forward move
+		boolean descending=false;
+		boolean closed=false;
+
 		public CursorImpl(TransactionImpl tx,TreeName treeName) {
 			this.treeName=treeName;
 			this.tx=tx;
-			rc=full();
-			iterator=rc.iterator();
+			//lazy: the first navigation decides which query to run, a seek must not pay for a partition scan
 		}
 
-		ResultSet full(){
-			return execute(
-				prepared.get("SELECT key,value FROM "+getTableName()+" WHERE baseDN=:baseDN and indexId=:indexId ORDER BY key").bind()
-					.setString("baseDN", treeName.getBaseDN()).setString("indexId", treeName.getIndexId())
-			);
+		ResultSet select(String condition,ByteSequence key,int rows){
+			queryCount++;
+			BoundStatement statement=prepared.get("SELECT key,value FROM "+tableName+" WHERE baseDN=:baseDN and indexId=:indexId"+condition).bind()
+				.setString("baseDN", treeName.getBaseDN()).setString("indexId", treeName.getIndexId())
+				.setPageSize(rows);
+			if (key!=null) {
+				statement=statement.setByteBuffer("key", ByteBuffer.wrap(key.toByteArray()));
+			}
+			return execute(statement);
 		}
-		
+
+		//runs a new page and positions the cursor on its first row
+		boolean slice(String condition,ByteSequence key){
+			rc=select(condition,key,pageSize);
+			iterator=rc.iterator();
+			descending=false;
+			if (iterator.hasNext()) {
+				current=iterator.next();
+				defined=true;
+				return true;
+			}
+			current=null;
+			defined=false;
+			return false;
+		}
+
+		boolean seek(ByteSequence key){
+			return slice(" and key>=:key ORDER BY key",key);
+		}
+
+		void growPage() {
+			pageSize=(int)Math.min(maxPageSize,2L*pageSize);
+		}
+
+		//serves a forward repositioning from the rows the driver already fetched: they are the
+		//sorted rows following the current one (blob clustering collates in unsigned byte order)
+		int forwardInPage(ByteSequence key,boolean exactMatch) {
+			if (!defined || descending || rc==null) {
+				return NEEDS_SEEK;
+			}
+			int cmp=key.compareTo(getKey());
+			if (cmp==0) {
+				return POSITIONED;
+			}
+			if (cmp<0) { //backward: only the server can rewind
+				return NEEDS_SEEK;
+			}
+			while (rc.getAvailableWithoutFetching()>0) {
+				current=iterator.next();
+				cmp=key.compareTo(getKey());
+				if (cmp==0) {
+					return POSITIONED;
+				}
+				if (cmp<0) { //walked past the key
+					if (exactMatch) {
+						defined=false; //the cursor stops just before this row
+						return MISSING;
+					}
+					return POSITIONED;
+				}
+			}
+			return PAGE_OUT;
+		}
+
 		@Override
 		public boolean next() {
-			try {
-				current=iterator.next();
-				return true;
-			}catch (NoSuchElementException e) {
-				current=null;
+			if (closed) {
+				return false;
 			}
-			return false;
+			if (current!=null && !defined) { //a failed positionToKey stopped just before this row
+				defined=true;
+				return true;
+			}
+			if (rc==null) { //lazy cursor: the first navigation runs the scan
+				return slice(" ORDER BY key",null);
+			}
+			if (!descending && rc.getAvailableWithoutFetching()>0) {
+				current=iterator.next();
+				defined=true;
+				return true;
+			}
+			if (current==null) { //exhausted or explicitly undefined: stay there
+				return false;
+			}
+			if (!descending && rc.isFullyFetched()) { //the server has nothing left either
+				current=null;
+				defined=false;
+				return false;
+			}
+			//page boundary: continue with a bigger slice starting right after the current key
+			growPage();
+			return slice(" and key>:key ORDER BY key",getKey());
 		}
 
 		@Override
 		public boolean isDefined() {
-			return current!=null;
+			return defined;
 		}
 
 		@Override
@@ -354,72 +462,101 @@
 			tx.delete(treeName, getKey());
 		}
 
+		//a closed cursor is undefined and every navigation on it returns false, like EmptyCursor
 		@Override
 		public void close() {
-			iterator=null;
+			closed=true;
+			iterator=Collections.emptyIterator();
 			current=null;
+			defined=false;
+			descending=false;
 			rc=null;
 		}
 
 
 		@Override
 		public boolean positionToKeyOrNext(ByteSequence key) {
-			if (!isDefined() || key.compareTo(getKey())<0) { //restart iterator
-				iterator=rc.iterator();
+			if (closed) {
+				return false;
 			}
-			while (iterator.hasNext()) {
-				current=iterator.next();
-				if (key.compareTo(getKey())<=0) {
-					return true;
-				}
+			final int served=forwardInPage(key,false);
+			if (served>=0) {
+				return served==POSITIONED;
 			}
-			current=null;
-			return false;
+			if (served==PAGE_OUT) { //the scan outran its page: the next slice should be bigger
+				growPage();
+			}
+			return seek(key);
 		}
-		
+
 		@Override
 		public boolean positionToKey(ByteSequence key) {
-			if (!isDefined() || key.compareTo(getKey())<0) {  //restart iterator
-				iterator=rc.iterator();
+			if (closed) {
+				return false;
 			}
-			if (isDefined() && key.compareTo(getKey())==0) {
+			final int served=forwardInPage(key,true);
+			if (served>=0) {
+				return served==POSITIONED;
+			}
+			if (served==PAGE_OUT) {
+				growPage();
+			}
+			if (seek(key) && key.compareTo(getKey())==0) {
 				return true;
 			}
-			while (iterator.hasNext()) {
-				current=iterator.next();
-				if (key.compareTo(getKey())==0) {
-					return true;
-				}
-			}
-			current=null;
+			//like jeb/pdb a miss leaves the cursor undefined; the row the seek landed on is the
+			//first key after the missing one, so next() resumes there instead of skipping it
+			defined=false;
 			return false;
 		}
 
-		
+
 		@Override
 		public boolean positionToLastKey() {
-			while (iterator.hasNext()) {
-				current=iterator.next();
+			if (closed) {
+				return false;
 			}
-			if (current!=null) {
+			rc=select(" ORDER BY key DESC LIMIT 1",null,1);
+			iterator=rc.iterator();
+			descending=true;
+			if (iterator.hasNext()) {
+				current=iterator.next(); //nothing follows the last key
+				defined=true;
 				return true;
 			}
+			current=null;
+			defined=false;
 			return false;
 		}
 
 		@Override
 		public boolean positionToIndex(int index) {
-			iterator=rc.iterator();  //restart iterator
-			int ct=0;
-			while(iterator.hasNext()){
-				current=iterator.next();
-				if (ct==index) {
-					return true;
-				}
-				ct++;
+			if (closed) {
+				return false;
 			}
-			current=null;
-			return false;
+			if (index<0) {
+				rc=null; //an invalid index resets the cursor: next() starts the scan again
+				iterator=null;
+				current=null;
+				defined=false;
+				descending=false;
+				return false;
+			}
+			//CQL has no offset clause: restart from the first row and skip. The rows that have to
+			//be walked are asked for in one page instead of one round trip per page
+			rc=select(" ORDER BY key",null,(int)Math.min(maxPageSize,Math.max(pageSize,index+1L)));
+			iterator=rc.iterator();
+			descending=false;
+			for (int ct=0;ct<=index;ct++) {
+				if (!iterator.hasNext()) {
+					current=null;
+					defined=false;
+					return false;
+				}
+				current=iterator.next();
+			}
+			defined=true;
+			return true;
 		}
 	}
 	
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java
index ed0baf3..9e784d3 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java
@@ -17,9 +17,22 @@
 
 import static org.mockito.Mockito.when;
 import static org.forgerock.opendj.config.ConfigurationMock.mockCfg;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
 
+import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.ByteStringBuilder;
 import org.forgerock.opendj.server.config.server.CASBackendCfg;
 import org.opends.server.backends.pluggable.PluggableBackendImplTestCase;
+import org.opends.server.backends.pluggable.spi.AccessMode;
+import org.opends.server.backends.pluggable.spi.Cursor;
+import org.opends.server.backends.pluggable.spi.ReadOperation;
+import org.opends.server.backends.pluggable.spi.ReadableTransaction;
+import org.opends.server.backends.pluggable.spi.TreeName;
+import org.opends.server.backends.pluggable.spi.WriteOperation;
+import org.opends.server.backends.pluggable.spi.WriteableTransaction;
 import org.testcontainers.DockerClientFactory;
 import org.testcontainers.containers.CassandraContainer;
 import org.testng.SkipException;
@@ -31,12 +44,18 @@
 import com.datastax.oss.driver.api.core.config.DriverConfigLoader;
 
 import java.net.InetSocketAddress;
+import java.util.NoSuchElementException;
 
 //docker run --rm -it -p 9042:9042 --name cassandra cassandra
 
-@Test
+//TestListener refuses a class that declares test methods of its own without sequential=true,
+//and the cursor tests below share one storage, so they must not be interleaved either
+@Test(groups = { "precommit", "pluggablebackend" }, sequential = true)
 public class TestCase extends PluggableBackendImplTestCase<CASBackendCfg> {
 
+	private static final String PAGE_INITIAL = "org.openidentityplatform.opendj.cassandra.fetchsize.initial";
+	private static final String PAGE_MAX = "org.openidentityplatform.opendj.cassandra.fetchsize";
+
 	CassandraContainer cassandraContainer;
 	@Override
 	protected Backend createBackend() {
@@ -76,4 +95,312 @@
 			cassandraContainer.close();
 		}
 	}
+
+	private static ByteString key(int i) {
+		return ByteString.valueOfUtf8(String.format("key%02d", i));
+	}
+
+	private static ByteString value(int i) {
+		return ByteString.valueOfUtf8("value" + i);
+	}
+
+	/**
+	 * A cursor reads its page sizes when the storage is built, so pinning them here keeps the query
+	 * counts below independent of the driver default (5000 rows) and of the storage defaults.
+	 */
+	private CASStorage openStorage(int initialPage, int maxPage) throws Exception {
+		System.setProperty(PAGE_INITIAL, String.valueOf(initialPage));
+		System.setProperty(PAGE_MAX, String.valueOf(maxPage));
+		try {
+			final CASStorage storage = new CASStorage(createBackendCfg(), null);
+			storage.open(AccessMode.READ_WRITE);
+			return storage;
+		} finally {
+			System.clearProperty(PAGE_INITIAL);
+			System.clearProperty(PAGE_MAX);
+		}
+	}
+
+	/** Rows left behind by an interrupted run would break the counts, so the tree starts empty. */
+	private static void fill(CASStorage storage, final TreeName tree, final int rows) throws Exception {
+		storage.write(new WriteOperation() {
+			@Override
+			public void run(WriteableTransaction txn) throws Exception {
+				txn.deleteTree(tree);
+				for (int i = 0; i < rows; i++) {
+					txn.put(tree, key(i), value(i));
+				}
+			}
+		});
+	}
+
+	private static void dropTree(CASStorage storage, final TreeName tree) {
+		try {
+			storage.write(new WriteOperation() {
+				@Override
+				public void run(WriteableTransaction txn) throws Exception {
+					txn.deleteTree(tree);
+				}
+			});
+		} catch (Exception e) { //a failed cleanup must be visible, but must not hide a test failure
+			System.err.println("cannot drop " + tree + ": " + e);
+		}
+	}
+
+	/**
+	 * The driver ResultSet is consumed once and cannot be rewound, so every repositioning that is
+	 * not a forward move within the already-fetched rows must run a new server-side slice query.
+	 * The old implementation "restarted" the iterator via rc.iterator(), which is a no-op: backward
+	 * repositioning returned the wrong row and positionToIndex counted from the current position.
+	 */
+	@Test
+	public void testCursorReposition() throws Exception {
+		final CASStorage storage = openStorage(32, 1000);
+		final TreeName tree = new TreeName("testCursorReposition", "tree");
+		try {
+			fill(storage, tree, 40);
+			storage.read(new ReadOperation<Void>() {
+				@Override
+				public Void run(ReadableTransaction txn) throws Exception {
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						final CASStorage.CursorImpl impl = (CASStorage.CursorImpl) cursor;
+						assertEquals(impl.queryCount, 0); // opening a cursor runs no query
+
+						assertTrue(cursor.positionToKeyOrNext(key(5))); // server-side seek
+						assertEquals(cursor.getKey(), key(5));
+						assertEquals(impl.queryCount, 1);
+						assertTrue(cursor.positionToKeyOrNext(key(5))); // same key: stays, no query
+						assertEquals(cursor.getKey(), key(5));
+						assertEquals(impl.queryCount, 1);
+						assertTrue(cursor.positionToKeyOrNext(key(9))); // forward: served from fetched rows
+						assertEquals(cursor.getKey(), key(9));
+						assertEquals(cursor.getValue(), value(9));
+						assertEquals(impl.queryCount, 1);
+
+						// backward: the old no-op "restart" returned the next remaining row instead
+						assertTrue(cursor.positionToKeyOrNext(key(2)));
+						assertEquals(cursor.getKey(), key(2));
+						assertEquals(cursor.getValue(), value(2));
+						assertTrue(cursor.positionToKeyOrNext(ByteString.valueOfUtf8("key021"))); // between rows
+						assertEquals(cursor.getKey(), key(3));
+
+						assertTrue(cursor.positionToKey(key(1))); // backward exact match
+						assertEquals(cursor.getKey(), key(1));
+						final long queries = impl.queryCount;
+						assertFalse(cursor.positionToKey(ByteString.valueOfUtf8("key011"))); // missing key
+						assertFalse(cursor.isDefined());
+						assertEquals(impl.queryCount, queries); // the miss was decided within the page
+						assertTrue(cursor.next()); // a miss stops just before the next key (like pdb)
+						assertEquals(cursor.getKey(), key(2));
+						assertTrue(cursor.positionToKey(key(1)));
+						assertTrue(cursor.next()); // next() continues right after the positioned key (DN2ID)
+						assertEquals(cursor.getKey(), key(2));
+
+						// positionToIndex counts from the first row, not from the current position
+						assertTrue(cursor.positionToIndex(0));
+						assertEquals(cursor.getKey(), key(0));
+						assertTrue(cursor.positionToIndex(39));
+						assertEquals(cursor.getKey(), key(39));
+						assertFalse(cursor.positionToIndex(40));
+
+						assertTrue(cursor.positionToLastKey()); // LIMIT 1 query, no partition scan
+						assertEquals(cursor.getKey(), key(39));
+						assertFalse(cursor.next());
+						assertTrue(cursor.positionToKeyOrNext(key(0))); // reposition after exhaustion
+						assertEquals(cursor.getKey(), key(0));
+						assertFalse(cursor.positionToKeyOrNext(ByteString.valueOfUtf8("key99"))); // beyond last
+
+						// VLVIndex.evaluateVLVRequestByAssertion: seek to the assertion, then to the start
+						assertTrue(cursor.positionToKeyOrNext(key(20)) && cursor.positionToIndex(0));
+						assertEquals(cursor.getKey(), key(0));
+					}
+
+					// EntryContainer.deleteSubtree/renameSubtree walk an ascending key list on one
+					// shared id2entry cursor: forward moves must be served from the fetched rows,
+					// otherwise every entry costs a page-sized slice of full entries
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						final CASStorage.CursorImpl impl = (CASStorage.CursorImpl) cursor;
+						for (int i = 0; i < 40; i++) {
+							assertTrue(cursor.positionToKey(key(i)), "missing " + key(i));
+							assertEquals(cursor.getValue(), value(i));
+						}
+						assertEquals(impl.queryCount, 2, "ascending walk took " + impl.queryCount + " queries");
+					}
+
+					// DN2ID.ChildrenCursor: reposition to currentKey+0x01 for every row; forward
+					// repositioning is served from the fetched rows, so the scan stays at ~2 queries
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						final CASStorage.CursorImpl impl = (CASStorage.CursorImpl) cursor;
+						assertTrue(cursor.positionToKeyOrNext(key(0)));
+						int rows = 1;
+						while (cursor.positionToKeyOrNext(
+								new ByteStringBuilder().appendBytes(cursor.getKey()).appendByte(0x01).toByteString())) {
+							rows++;
+						}
+						assertEquals(rows, 40);
+						assertEquals(impl.queryCount, 3, "sibling scan took " + impl.queryCount + " queries");
+					}
+					return null;
+				}
+			});
+		} finally {
+			dropTree(storage, tree);
+			storage.close();
+		}
+	}
+
+	/**
+	 * Everything a cursor does once its page runs out - continuing a scan, falling back to a slice,
+	 * growing the page - only runs on trees bigger than one page, so the page is pinned small here.
+	 * With the driver default of 5000 rows none of these branches would be exercised at all.
+	 */
+	@Test
+	public void testCursorPagingAcrossPages() throws Exception {
+		final CASStorage storage = openStorage(4, 8);
+		final TreeName tree = new TreeName("testCursorPaging", "tree");
+		try {
+			fill(storage, tree, 40);
+			storage.read(new ReadOperation<Void>() {
+				@Override
+				public Void run(ReadableTransaction txn) throws Exception {
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						final CASStorage.CursorImpl impl = (CASStorage.CursorImpl) cursor;
+						int rows = 0;
+						while (cursor.next()) { // the scan continues across page boundaries
+							assertEquals(cursor.getKey(), key(rows));
+							assertEquals(cursor.getValue(), value(rows));
+							rows++;
+						}
+						assertEquals(rows, 40);
+						assertFalse(cursor.next()); // and stops for good at the end of the partition
+						assertEquals(impl.pageSize, 8); // the page grew, but not past the maximum
+						assertTrue(impl.queryCount <= 8, "scan took " + impl.queryCount + " queries");
+					}
+
+					// forward repositioning falls back to a server-side slice when the page runs out
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						final CASStorage.CursorImpl impl = (CASStorage.CursorImpl) cursor;
+						assertTrue(cursor.positionToKeyOrNext(key(0)));
+						int rows = 1;
+						while (cursor.positionToKeyOrNext(
+								new ByteStringBuilder().appendBytes(cursor.getKey()).appendByte(0x01).toByteString())) {
+							assertEquals(cursor.getKey(), key(rows));
+							rows++;
+						}
+						assertEquals(rows, 40);
+						assertTrue(impl.queryCount <= 9, "sibling scan took " + impl.queryCount + " queries");
+					}
+
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						// rows far beyond the first page are still reachable in one seek
+						assertTrue(cursor.positionToKey(key(37)));
+						assertEquals(cursor.getValue(), value(37));
+						assertTrue(cursor.next());
+						assertEquals(cursor.getKey(), key(38));
+						assertTrue(cursor.positionToKeyOrNext(ByteString.valueOfUtf8("key385")));
+						assertEquals(cursor.getKey(), key(39));
+						assertTrue(cursor.positionToIndex(39));
+						assertEquals(cursor.getKey(), key(39));
+						assertTrue(cursor.positionToIndex(20));
+						assertEquals(cursor.getKey(), key(20));
+						assertTrue(cursor.positionToLastKey());
+						assertEquals(cursor.getKey(), key(39));
+						assertFalse(cursor.next());
+					}
+
+					// DN2ID.openCursor0: position, then iterate with next() over several pages
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						assertTrue(cursor.positionToKey(key(2)));
+						for (int i = 3; i < 40; i++) {
+							assertTrue(cursor.next());
+							assertEquals(cursor.getKey(), key(i));
+						}
+						assertFalse(cursor.next());
+					}
+					return null;
+				}
+			});
+		} finally {
+			dropTree(storage, tree);
+			storage.close();
+		}
+	}
+
+	/** Serving forward repositioning from fetched rows relies on the unsigned blob clustering order. */
+	@Test
+	public void testCursorKeyOrderIsUnsigned() throws Exception {
+		final CASStorage storage = openStorage(32, 1000);
+		final TreeName tree = new TreeName("testCursorOrder", "tree");
+		final ByteString low = ByteString.valueOfBytes(new byte[] { 0x7F });
+		final ByteString high = ByteString.valueOfBytes(new byte[] { (byte) 0x80, 0x01 });
+		try {
+			storage.write(new WriteOperation() {
+				@Override
+				public void run(WriteableTransaction txn) throws Exception {
+					txn.deleteTree(tree);
+					txn.put(tree, low, value(1));
+					txn.put(tree, high, value(2));
+				}
+			});
+			storage.read(new ReadOperation<Void>() {
+				@Override
+				public Void run(ReadableTransaction txn) throws Exception {
+					try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+						final CASStorage.CursorImpl impl = (CASStorage.CursorImpl) cursor;
+						// with a signed collation 0x80 would sort before 0x7F and these would fail
+						assertTrue(cursor.next());
+						assertEquals(cursor.getKey(), low);
+						final long queries = impl.queryCount;
+						// {0x80} is above {0x7F} and below {0x80,0x01}, so this is a forward move served
+						// from the fetched rows: it is the client-side comparison that is checked here
+						assertTrue(cursor.positionToKeyOrNext(ByteString.valueOfBytes(new byte[] { (byte) 0x80 })));
+						assertEquals(cursor.getKey(), high);
+						assertEquals(impl.queryCount, queries);
+						assertFalse(cursor.next()); // {0x80,0x01} is the last key
+						assertTrue(cursor.positionToLastKey());
+						assertEquals(cursor.getKey(), high);
+					}
+					return null;
+				}
+			});
+		} finally {
+			dropTree(storage, tree);
+			storage.close();
+		}
+	}
+
+	/** A closed cursor is undefined and every navigation on it returns false, like EmptyCursor. */
+	@Test
+	public void testCursorAfterClose() throws Exception {
+		final CASStorage storage = openStorage(32, 1000);
+		final TreeName tree = new TreeName("testCursorClose", "tree");
+		try {
+			fill(storage, tree, 4);
+			storage.read(new ReadOperation<Void>() {
+				@Override
+				public Void run(ReadableTransaction txn) throws Exception {
+					final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree);
+					assertTrue(cursor.positionToKeyOrNext(key(0)));
+					cursor.close();
+					assertFalse(cursor.isDefined());
+					assertFalse(cursor.next());
+					assertFalse(cursor.positionToKey(key(0)));
+					assertFalse(cursor.positionToKeyOrNext(key(0)));
+					assertFalse(cursor.positionToLastKey());
+					assertFalse(cursor.positionToIndex(0));
+					try {
+						cursor.getKey();
+						fail("a closed cursor has no key");
+					} catch (NoSuchElementException expected) {
+						// a closed cursor has no current row
+					}
+					cursor.close(); // closing twice is not an error
+					return null;
+				}
+			});
+		} finally {
+			dropTree(storage, tree);
+			storage.close();
+		}
+	}
 }

--
Gitblit v1.10.0