GP-0 DB set null state improvements for Fields and SparseRecord

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

@@ -174,6 +174,10 @@ public final class BooleanField extends PrimitiveField {
 
 	@Override
 	public void setBinaryData(byte[] bytes) {
+		if (bytes == null) {
+			setNull();
+			return;
+		}
 		if (bytes.length != 1) {
 			throw new IllegalFieldAccessException();
 		}

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

@@ -184,6 +184,10 @@ public final class ByteField extends PrimitiveField {
 
 	@Override
 	public void setBinaryData(byte[] bytes) {
+		if (bytes == null) {
+			setNull();
+			return;
+		}
 		if (bytes.length != 1) {
 			throw new IllegalFieldAccessException();
 		}

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

@@ -162,6 +162,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Get a copy of the specified field value.
 	 * @param columnIndex field index
 	 * @return Field field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 */
 	public Field getFieldValue(int columnIndex) {
 		Field f = fieldValues[columnIndex];
@@ -172,6 +173,8 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the field value for the specified field.
 	 * @param colIndex field index
 	 * @param value field value (null permitted for sparse column only)
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
+	 * @throws IllegalArgumentException if value type does not match column field type.
 	 */
 	public void setField(int colIndex, Field value) {
 		if (fieldValues[colIndex].getFieldType() != value.getFieldType()) {
@@ -186,6 +189,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * modified.
 	 * @param columnIndex field index
 	 * @return Field
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 */
 	Field getField(int columnIndex) {
 		return fieldValues[columnIndex];
@@ -206,6 +210,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * @param columnIndex field index
 	 * @param field field value to compare with
 	 * @return true if the fields are equal, else false.
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 */
 	public boolean fieldEquals(int columnIndex, Field field) {
 		return fieldValues[columnIndex].equals(field);
@@ -218,6 +223,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * @return 0 if equals, a negative number if this record's field is less
 	 * than the specified value, or a positive number if this record's field is
 	 * greater than the specified value.
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 */
 	public int compareFieldTo(int columnIndex, Field value) {
 		return fieldValues[columnIndex].compareTo(value);
@@ -264,6 +270,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Get the long value for the specified field.
 	 * @param colIndex field index
 	 * @return field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support long data access
 	 */
 	public long getLongValue(int colIndex) {
@@ -274,6 +281,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the long value for the specified field.
 	 * @param colIndex field index
 	 * @param value field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support long data access
 	 */
 	public void setLongValue(int colIndex, long value) {
@@ -285,6 +293,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Get the integer value for the specified field.
 	 * @param colIndex field index
 	 * @return field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support integer data access
 	 */
 	public int getIntValue(int colIndex) {
@@ -295,6 +304,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the integer value for the specified field.
 	 * @param colIndex field index
 	 * @param value field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support integer data access
 	 */
 	public void setIntValue(int colIndex, int value) {
@@ -306,6 +316,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Get the short value for the specified field.
 	 * @param colIndex field index
 	 * @return field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support short data access
 	 */
 	public short getShortValue(int colIndex) {
@@ -316,6 +327,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the short value for the specified field.
 	 * @param colIndex field index
 	 * @param value field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support short data access
 	 */
 	public void setShortValue(int colIndex, short value) {
@@ -327,6 +339,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Get the byte value for the specified field.
 	 * @param colIndex field index
 	 * @return field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support byte data access
 	 */
 	public byte getByteValue(int colIndex) {
@@ -337,6 +350,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the byte value for the specified field.
 	 * @param colIndex field index
 	 * @param value field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support byte data access
 	 */
 	public void setByteValue(int colIndex, byte value) {
@@ -348,6 +362,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Get the boolean value for the specified field.
 	 * @param colIndex field index
 	 * @return field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support boolean data access
 	 */
 	public boolean getBooleanValue(int colIndex) {
@@ -358,6 +373,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the boolean value for the specified field.
 	 * @param colIndex field index
 	 * @param value field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support boolean data access
 	 */
 	public void setBooleanValue(int colIndex, boolean value) {
@@ -369,6 +385,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Get the binary data array for the specified field.
 	 * @param colIndex field index
 	 * @return field data
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support binary data access
 	 */
 	public byte[] getBinaryData(int colIndex) {
@@ -379,19 +396,39 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the binary data array for the specified field.
 	 * @param colIndex field index
 	 * @param bytes field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support binary data access
 	 * or incorrect number of bytes provided
 	 */
 	public void setBinaryData(int colIndex, byte[] bytes) {
 		dirty = true;
-		invalidateLength();
+		Field f = fieldValues[colIndex];
-		fieldValues[colIndex].setBinaryData(bytes);
+		if (f.isVariableLength()) {
+			invalidateLength();
+		}
+		f.setBinaryData(bytes);
+	}
+
+	/**
+	 * Set the field to a null state.  For a non-sparse fixed-length column field this will
+	 * set the the value to zero and the null state will not be persisted when stored.
+	 * @param colIndex field index
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
+	 */
+	public void setNull(int colIndex) {
+		dirty = true;
+		Field f = fieldValues[colIndex];
+		if (f.isVariableLength()) {
+			invalidateLength();
+		}
+		f.setNull();
 	}
 
 	/**
 	 * Get the string value for the specified field.
 	 * @param colIndex field index
 	 * @return field data
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support string data access
 	 */
 	public String getString(int colIndex) {
@@ -402,6 +439,7 @@ public class DBRecord implements Comparable<DBRecord> {
 	 * Set the string value for the specified field.
 	 * @param colIndex field index
 	 * @param str field value
+	 * @throws ArrayIndexOutOfBoundsException if invalid columnIndex is specified
 	 * @throws IllegalFieldAccessException if field does support string data access
 	 */
 	public void setString(int colIndex, String str) {

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

@@ -162,8 +162,11 @@ public class FixedField10 extends FixedField {
 
 	@Override
 	public void setBinaryData(byte[] d) {
-		if (d == null || d.length != 10) {
+		if (d == null) {
-			// null value not permitted although null state is (see setNull())
+			setNull();
+			return;
+		}
+		if (d.length != 10) {
 			throw new IllegalArgumentException("Invalid FixedField10 data length");
 		}
 		updatingValue();

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

@@ -185,6 +185,10 @@ public final class IntField extends PrimitiveField {
 
 	@Override
 	public void setBinaryData(byte[] bytes) {
+		if (bytes == null) {
+			setNull();
+			return;
+		}
 		if (bytes.length != 4) {
 			throw new IllegalFieldAccessException();
 		}

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

@@ -179,6 +179,10 @@ public final class LongField extends PrimitiveField {
 
 	@Override
 	public void setBinaryData(byte[] bytes) {
+		if (bytes == null) {
+			setNull();
+			return;
+		}
 		if (bytes.length != 8) {
 			throw new IllegalFieldAccessException();
 		}

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

@@ -184,6 +184,10 @@ public final class ShortField extends PrimitiveField {
 
 	@Override
 	public void setBinaryData(byte[] bytes) {
+		if (bytes == null) {
+			setNull();
+			return;
+		}
 		if (bytes.length != 2) {
 			throw new IllegalFieldAccessException();
 		}

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

@@ -93,13 +93,51 @@ public class SparseRecord extends DBRecord {
 		dirty = false;
 	}
 
-	private boolean changeInSparseStorage(int colIndex, long newValue) {
+	/**
+	 * Check for a change in a sparse column's storage size when setting a non-null primitive value.
+	 * All primitive value storage is fixed-length and only varies for a sparse column when
+	 * transitioning between a null and non-null state.
+	 * @param colIndex field column index within this record.
+	 * @return true for a sparse column which is transitioning between a null and non-null state,
+	 * else false.
+	 */
+	private boolean changeInSparsePrimitiveStorage(int colIndex) {
 		if (!schema.isSparseColumn(colIndex)) {
 			return false;
 		}
-		boolean oldSparse = getField(colIndex).isNull();
+		return getField(colIndex).isNull();
-		boolean newSparse = newValue == 0;
+	}
-		return oldSparse != newSparse;
+
+	/**
+	 * Check for a change in a sparse column's storage size when setting a binary value.
+	 * This method only checks for a sparse column's transition between a null and non-null state.
+	 * While this is the only length change consideration needed for a fixed-length field (e.g. 
+	 * {@link FixedField}, {@link PrimitiveField}), record length invalidation due to a change
+	 * in variable-length {@link Field} data must be handled separately.
+	 * @param colIndex field column index within this record.
+	 * @return true for a sparse column which is transitioning between a null and non-null state,
+	 * else false.
+	 */
+	private boolean changeInSparseStorage(int colIndex, byte[] newValue) {
+		if (!schema.isSparseColumn(colIndex)) {
+			return false;
+		}
+		boolean oldIsNull = getField(colIndex).isNull();
+		boolean newIsNull = newValue == null;
+		return oldIsNull != newIsNull;
+	}
+
+	/**
+	 * Check for a change in a sparse column's storage size when setting a column to the null state.
+	 * @param colIndex field column index within this record.
+	 * @return true for a sparse column which is transitioning between a null and non-null state,
+	 * else false.
+	 */
+	private boolean changeInSparseNullStorage(int colIndex) {
+		if (!schema.isSparseColumn(colIndex)) {
+			return false;
+		}
+		return !getField(colIndex).isNull();
 	}
 
 	@Override
@@ -116,7 +154,7 @@ public class SparseRecord extends DBRecord {
 
 	@Override
 	public void setLongValue(int colIndex, long value) {
-		if (changeInSparseStorage(colIndex, value)) {
+		if (changeInSparsePrimitiveStorage(colIndex)) {
 			invalidateLength();
 		}
 		super.setLongValue(colIndex, value);
@@ -124,7 +162,7 @@ public class SparseRecord extends DBRecord {
 
 	@Override
 	public void setIntValue(int colIndex, int value) {
-		if (changeInSparseStorage(colIndex, value)) {
+		if (changeInSparsePrimitiveStorage(colIndex)) {
 			invalidateLength();
 		}
 		super.setIntValue(colIndex, value);
@@ -132,7 +170,7 @@ public class SparseRecord extends DBRecord {
 
 	@Override
 	public void setShortValue(int colIndex, short value) {
-		if (changeInSparseStorage(colIndex, value)) {
+		if (changeInSparsePrimitiveStorage(colIndex)) {
 			invalidateLength();
 		}
 		super.setShortValue(colIndex, value);
@@ -140,7 +178,7 @@ public class SparseRecord extends DBRecord {
 
 	@Override
 	public void setByteValue(int colIndex, byte value) {
-		if (changeInSparseStorage(colIndex, value)) {
+		if (changeInSparsePrimitiveStorage(colIndex)) {
 			invalidateLength();
 		}
 		super.setByteValue(colIndex, value);
@@ -148,10 +186,25 @@ public class SparseRecord extends DBRecord {
 
 	@Override
 	public void setBooleanValue(int colIndex, boolean value) {
-		if (changeInSparseStorage(colIndex, value ? 1 : 0)) {
+		if (changeInSparsePrimitiveStorage(colIndex)) {
 			invalidateLength();
 		}
 		super.setBooleanValue(colIndex, value);
 	}
 
+	@Override
+	public void setBinaryData(int colIndex, byte[] bytes) {
+		if (changeInSparseStorage(colIndex, bytes)) {
+			invalidateLength();
+		}
+		super.setBinaryData(colIndex, bytes);
+	}
+
+	@Override
+	public void setNull(int colIndex) {
+		if (changeInSparseNullStorage(colIndex)) {
+			invalidateLength();
+		}
+		super.setNull(colIndex);
+	}
 }

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

@@ -179,11 +179,11 @@ public final class StringField extends Field {
 	@Override
 	public void setBinaryData(byte[] bytes) {
 		checkImmutable();
+		this.bytes = bytes;
 		if (bytes == null) {
 			str = null;
 		}
 		else {
-			this.bytes = bytes;
 			try {
 				str = new String(bytes, ENCODING);
 			}

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

@@ -170,6 +170,7 @@ public class DBFixedKeySparseIndexedTableTest extends AbstractGenericTest {
 					f = f.getMaxValue();
 				}
 				r.setField(i, f);
+				assertFalse(f.isNull());
 			}
 
 			// set min value all fields before i
@@ -182,6 +183,7 @@ public class DBFixedKeySparseIndexedTableTest extends AbstractGenericTest {
 					f = f.getMinValue();
 				}
 				r.setField(m, f);
+				assertFalse(f.isNull());
 			}
 
 //			// NOTE: sparse columns default to a null state if not explicitly set