Corrected full reverse index iteration

Ghidra/Framework/DB/src/main/java/db/FieldIndexTable.java

@@ -497,7 +497,7 @@ public class FieldIndexTable extends IndexTable {
 		 * @param minValue minimum index value or null if no minimum
 		 * @param maxValue maximum index value or null if no maximum
 		 * @param startValue starting index value.
-		 * @param primaryKey starting primary key value.
+		 * @param primaryKey starting primary key value (ignored if startValue is null).
 		 * @param after if true iterator is positioned immediately after 
 		 * the startValue/primaryKey,
 		 * otherwise immediately before.
@@ -505,17 +505,26 @@ public class FieldIndexTable extends IndexTable {
 		 */
 		PrimaryKeyIterator(Field minValue, Field maxValue, Field startValue, long primaryKey,
 				boolean after) throws IOException {
+
 			min = minValue != null ? IndexField.getIndexField(minValue, Long.MIN_VALUE) : null;
 			max = maxValue != null ? IndexField.getIndexField(maxValue, Long.MAX_VALUE) : null;
-			IndexField start =
-				startValue != null ? IndexField.getIndexField(startValue, primaryKey) : null;
 
-			indexIterator = indexTable.fieldKeyIterator(min, max, start);
+			IndexField start = null;
-
+			if (after && startValue == null && maxValue == null) {
-			if (indexIterator.hasNext()) {
+				indexIterator = indexTable.fieldKeyIterator(min, max, !after);
-				Field f = indexIterator.next();
+				if (indexIterator.hasNext()) {
-				if (!after || !f.equals(start)) {
+					indexIterator.next(); // must step beyond max indexed value
-					indexIterator.previous();
+				}
+			}
+			else {
+				start =
+					startValue != null ? IndexField.getIndexField(startValue, primaryKey) : null;
+				indexIterator = indexTable.fieldKeyIterator(min, max, start);
+				if (indexIterator.hasNext()) {
+					Field f = indexIterator.next();
+					if (!after || !f.equals(start)) {
+						indexIterator.previous();
+					}
 				}
 			}
 		}

Ghidra/Framework/DB/src/main/java/db/IndexTable.java

@@ -300,9 +300,9 @@ abstract class IndexTable {
 	/**
 	 * Iterate over all primary keys sorted based upon the associated index key.
 	 * The iterator is limited to range of index keys of minField through maxField, inclusive.
-	 * If atMin is true, the iterator is initially positioned before the first index 
+	 * If before is true, the iterator is initially positioned before the first index 
 	 * buffer whose index key is greater than or equal to the specified minField value. 
-	 * If atMin is false, the iterator is initially positioned after the first index 
+	 * If before is false, the iterator is initially positioned after the first index 
 	 * buffer whose index key is less than or equal to the specified maxField value. 
 	 * @param minField minimum index key value
 	 * @param maxField maximum index key value
@@ -312,6 +312,13 @@ abstract class IndexTable {
 	 * @throws IOException thrown if IO error occurs
 	 */
 	DBLongIterator keyIterator(Field minField, Field maxField, boolean before) throws IOException {
+
+		Field startField = before ? minField : maxField;
+
+		if (startField == null && !before) {
+
+		}
+
 		return new PrimaryKeyIterator(minField, maxField, before ? minField : maxField,
 			before ? Long.MIN_VALUE : Long.MAX_VALUE, !before);
 	}

Ghidra/Framework/DB/src/test/java/db/DBIndexedTableTest.java

@@ -26,7 +26,6 @@ import org.junit.*;
 
 import db.buffers.*;
 import generic.test.AbstractGenericTest;
-import ghidra.util.Msg;
 import ghidra.util.exception.CancelledException;
 import utilities.util.FileUtilities;
 
@@ -616,7 +615,16 @@ public class DBIndexedTableTest extends AbstractGenericTest {
 				Record rec = iter.next();
 				assertEquals(recs[recIx++], rec);
 			}
-			assertEquals(recIx, maxIx + 1);
+			assertEquals(maxIx + 1, recIx);
+
+			// Full forward record iterator
+			recIx = 0;
+			iter = table.indexIterator(colIx, null, null, true);
+			while (iter.hasNext()) {
+				Record rec = iter.next();
+				assertEquals(recs[recIx++], rec);
+			}
+			assertEquals(recordCnt, recIx);
 
 			// Backward Range iterator
 			minIx = findStart(recs, recordCnt / 10, colIx);
@@ -628,7 +636,16 @@ public class DBIndexedTableTest extends AbstractGenericTest {
 				Record rec = iter.previous();
 				assertEquals(recs[recIx--], rec);
 			}
-			assertEquals(recIx, minIx - 1);
+			assertEquals(minIx - 1, recIx);
+
+			// Full reverse record iterator
+			recIx = recordCnt - 1;
+			iter = table.indexIterator(colIx, null, null, false);
+			while (iter.hasPrevious()) {
+				Record rec = iter.previous();
+				assertEquals(recs[recIx--], rec);
+			}
+			assertEquals(-1, recIx);
 
 			if (recs[0].getField(colIx) instanceof LongField) {
 
@@ -1076,7 +1093,7 @@ public class DBIndexedTableTest extends AbstractGenericTest {
 
 	@Test
 	public void testRandomRecordIterator() throws IOException {
-		iterateRecords(false, DBTestUtils.ALL_TYPES, ITER_REC_CNT, 0, 1, false);
+		iterateRecords(false, DBTestUtils.ALL_TYPES, ITER_REC_CNT, 0, -1, false);
 	}
 
 	@Test
@@ -1136,7 +1153,7 @@ public class DBIndexedTableTest extends AbstractGenericTest {
 
 	@Test
 	public void testRandomRecordIteratorUndoRedo() throws IOException {
-		iterateRecords(false, DBTestUtils.ALL_TYPES, ITER_REC_CNT, 0, 1, true);
+		iterateRecords(false, DBTestUtils.ALL_TYPES, ITER_REC_CNT, 0, -1, true);
 	}
 
 	@Test
@@ -1203,53 +1220,4 @@ public class DBIndexedTableTest extends AbstractGenericTest {
 		}
 	}
 
-	@Test
-	public void testReverseIndexRecordIterator() throws IOException {
-		long txId = dbh.startTransaction();
-		Table table =
-			DBTestUtils.createLongKeyTable(dbh, table1Name, DBTestUtils.SINGLE_BINARY, true);
-
-		while (table.getRecordCount() < 10) {
-			try {
-				DBTestUtils.createLongKeyRecord(table, true, 16, true);
-			}
-			catch (DuplicateKeyException e) {
-				Msg.info(this, "What are the odds?:" + e);
-			}
-		}
-		dbh.endTransaction(txId, true);
-
-		// Check my sanity. Does it work in the forward direction?
-		RecordIterator fit = table.indexIterator(0, null, null, false);
-		ArrayList<byte[]> fCol = new ArrayList<>(10);
-		while (fit.hasNext()) {
-			Record rec = fit.next();
-			fCol.add(rec.getBinaryData(0));
-		}
-		assertEquals(10, fCol.size()); // Did I get everyone?
-
-		byte[] last = null;
-		for (byte[] bin : fCol) { // Is each greater than or equal to the last?
-			if (last != null) {
-				assertTrue(new BinaryField(bin).compareTo(new BinaryField(last)) >= 0);
-			}
-			last = bin;
-		}
-
-		RecordIterator rit = table.indexIterator(0, null, null, false);
-		ArrayList<byte[]> rCol = new ArrayList<>(10);
-		while (rit.hasPrevious()) {
-			Record rec = rit.previous();
-			rCol.add(rec.getBinaryData(0));
-		}
-		assertEquals(10, rCol.size()); // Did I get everyone?
-
-		last = null;
-		for (byte[] bin : rCol) {
-			if (last != null) { // Is each less than or equal to the last?
-				assertTrue(new BinaryField(bin).compareTo(new BinaryField(last)) <= 0);
-			}
-			last = bin;
-		}
-	}
 }

Ghidra/Framework/DB/src/test/java/db/DBTestUtils.java

@@ -372,11 +372,15 @@ public class DBTestUtils {
 				rec.setLongValue(i, random.nextLong());
 			}
 			else if (fields[i] instanceof StringField) {
-				if (varDataSize < 0) {
+				int size = varDataSize;
+				if (size < 0) {
+					size = random.nextInt(6) - 1;
+				}
+				if (size < 0) {
 					rec.setString(i, null);
 				}
 				else {
-					char[] chars = new char[varDataSize];
+					char[] chars = new char[size];
 					for (int n = 0; n < chars.length; n++) {
 						chars[n] = (char) (random.nextInt() & 0x7fff);
 					}
@@ -385,11 +389,15 @@ public class DBTestUtils {
 				}
 			}
 			else if (fields[i] instanceof BinaryField) {
-				if (varDataSize < 0) {
+				int size = varDataSize;
+				if (size < 0) {
+					size = random.nextInt(6) - 1;
+				}
+				if (size < 0) {
 					rec.setBinaryData(i, null);
 				}
 				else {
-					byte[] bytes = new byte[varDataSize];
+					byte[] bytes = new byte[size];
 					random.nextBytes(bytes);
 					rec.setBinaryData(i, bytes);
 				}