This PR fixes how batch tables choose noData values when upgrading to EXT_structural_metadata. Specifically,
For signed ints, the order of checks in getSentinelValue prevented -1 from being chosen as a sentinel value if 0 was taken. The order has been changed to account for this.
Unsigned ints will be upgraded to the next biggest signed int (if possible) if 0 cannot act as a sentinel value. This allows -1 to be used as its sentinel value instead.
If no sentinel value is available for a property that has null, explicitly force it to a string property.
Without these checks, there is some unpredictable behavior because some of the code assumes a valid sentinel value exists, leading to dereferenced nullopts 😬
kring
commented
7 months ago
This PR fixes how batch tables choose
noDatavalues when upgrading toEXT_structural_metadata. Specifically,ints, the order of checks ingetSentinelValueprevented -1 from being chosen as a sentinel value if 0 was taken. The order has been changed to account for this.ints will be upgraded to the next biggest signedint(if possible) if 0 cannot act as a sentinel value. This allows -1 to be used as its sentinel value instead.null, explicitly force it to a string property.Without these checks, there is some unpredictable behavior because some of the code assumes a valid sentinel value exists, leading to dereferenced
nullopts 😬