xiangfu0 commented on code in PR #19479:
URL: https://github.com/apache/pinot/pull/19479#discussion_r4091517826
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -104,19 +133,29 @@ public class ColumnMetadataImpl implements ColumnMetadata
{
private final FieldSpec _fieldSpec;
private final int _totalDocs;
private final int _cardinality;
+ private final int _totalNumberOfEntries;
+ private final int _maxNumberOfMultiValues;
+ /// Two words with a use that depends on whether the stored type is fixed
width, which is exactly the condition
+ /// under which the other use is dead:
+ /// - fixed-width stored type (INT, LONG, FLOAT, DOUBLE, and
BOOLEAN/TIMESTAMP through their stored type): the raw
+ /// bits of the min and max value, boxed on demand by [#getMinValue()] /
[#getMaxValue()], with presence carried
+ /// by [#MIN_VALUE_IN_WORD] / [#MAX_VALUE_IN_WORD]. The element lengths
are dead here because [Builder#build()]
+ /// pins them to `storedType.size()`.
+ /// - otherwise: `_minWord` packs `lengthOfShortestElement` (high half) and
`lengthOfLongestElement` (low half),
+ /// `_maxWord` holds `maxRowLengthInBytes`. The value words are dead here
because a STRING, BYTES, BIG_DECIMAL or
Review Comment:
**MINOR (doc precision):** COMPLEX never has a min/max (`:530-532` flags it
invalid), so it does not belong in the "…or COMPLEX min/max is an object" list.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -74,24 +74,53 @@
/// one instance per distinct spec instead of retaining its own. Callers must
treat shared specs and their nested
/// values as read-only. Deserialize [FieldSpec#toJsonObject()] to make a copy
before editing a spec.
///
-/// The object layout is kept at 72 bytes for an ordinary column for the same
reason: the six booleans and the
-/// forward-index encoding are packed into one [#_flags] byte, and the refs
only a partitioned column or an OPEN_STRUCT
-/// parent/child carries (partition function and partitions, parent column,
sparse keys) live in a lazily allocated
-/// [Extras] holder that stays `null` for every other column. The compression
stats stay a direct ref because the
-/// segment creator writes them for every raw column. None of this is visible
through the public getters, so the
-/// `/tables/{table}/segments/{segment}/metadata` payload (bean-serialized
from the getters) is unchanged.
+/// The object layout is kept at 72 bytes for every column for the same
reason: the six booleans, the forward-index
+/// encoding, the two min/max representation bits and `bitsPerElement` are
packed into one [#_flags] int; the refs
+/// only a partitioned column or an OPEN_STRUCT parent/child carries
(partition function and partitions, parent
+/// column, sparse keys) live in a lazily allocated [Extras] holder that stays
`null` for every other column; and the
+/// three element-length ints share their two words with the numeric min/max
(see [#_minWord]). The compression stats
+/// stay a direct ref because the segment creator writes them for every raw
column. None of this is visible through
+/// the public getters, so the `/tables/{table}/segments/{segment}/metadata`
payload (bean-serialized from the
+/// getters) is unchanged.
+///
+/// | bytes | field(s) |
+/// |------:|----------|
+/// | 12 | object header |
+/// | 16 | `_minWord`, `_maxWord` |
+/// | 16 | `_totalDocs`, `_cardinality`, `_totalNumberOfEntries`,
`_maxNumberOfMultiValues` |
+/// | 4 | `_flags` |
+/// | 24 | `_fieldSpec`, `_minValue`, `_maxValue`, `_extras`,
`_compressionMetadata`, `_indexTypeSizes` |
+/// | 72 | total |
+///
+/// The saving over the eight ints, six refs and flags byte this replaced is
not in the object itself, which is the
+/// same 72 bytes, but in what it no longer retains: a fixed-width column
holds no box per min/max value, which is
Review Comment:
**MINOR (doc precision):** "~32 bytes and two surviving objects per numeric
column" is exact for INT/FLOAT (`Integer`/`Float` are 16 B each); LONG/DOUBLE
boxes are 24 B each, i.e. 48 B.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -989,12 +1079,60 @@ public ColumnMetadataImpl build() {
if (_autoGenerated) {
flags |= AUTO_GENERATED;
}
- return new ColumnMetadataImpl(_fieldSpec, _totalDocs, _cardinality,
_minValue, _maxValue,
- _lengthOfShortestElement, _lengthOfLongestElement,
_totalNumberOfEntries, _maxNumberOfMultiValues,
- _maxRowLengthInBytes, _bitsPerElement, flags,
- Extras.create(_partitionFunction, _partitions, _parentColumn,
_sparseKeys),
+ boolean encodableBitsPerElement =
+ _bitsPerElement >= UNAVAILABLE && _bitsPerElement <=
MAX_ENCODABLE_BITS_PER_ELEMENT;
+ flags |= (encodableBitsPerElement ? _bitsPerElement + 1 :
BITS_PER_ELEMENT_IN_EXTRAS) << BITS_PER_ELEMENT_SHIFT;
+
+ // Fill the two words with whichever of the two uses this column has
(see ColumnMetadataImpl#_minWord).
+ long minWord = 0;
+ long maxWord = 0;
+ Comparable<?> minValue = _minValue;
+ Comparable<?> maxValue = _maxValue;
+ if (storedType.isFixedWidth()) {
+ Long minBits = toValueWord(storedType, minValue);
+ if (minBits != null) {
+ minWord = minBits;
+ minValue = null;
+ flags |= MIN_VALUE_IN_WORD;
+ }
+ Long maxBits = toValueWord(storedType, maxValue);
+ if (maxBits != null) {
+ maxWord = maxBits;
+ maxValue = null;
+ flags |= MAX_VALUE_IN_WORD;
+ }
+ } else {
+ minWord = ((long) _lengthOfShortestElement << 32) |
(_lengthOfLongestElement & 0xffffffffL);
+ maxWord = _maxRowLengthInBytes & 0xffffffffL;
+ }
+
+ return new ColumnMetadataImpl(_fieldSpec, _totalDocs, _cardinality,
_totalNumberOfEntries,
+ _maxNumberOfMultiValues, minWord, maxWord, minValue, maxValue, flags,
+ Extras.create(_partitionFunction, _partitions, _parentColumn,
_sparseKeys,
+ encodableBitsPerElement ? UNAVAILABLE : _bitsPerElement),
CompressionMetadata.create(_uncompressedValueSizeInBytes,
_forwardIndexChunkCompressionType,
_dictionaryUncompressedValueSizeInBytes));
}
+
+ /// Returns the raw bits of a min/max value of a fixed-width stored type,
or `null` when there is no value or the
+ /// value is not the box class of the stored type (in which case it stays
an object ref, so an unexpected type
+ /// from a [Builder] caller is preserved rather than dropped or
mistranslated). FLOAT and DOUBLE go through
+ /// [Float#floatToIntBits] / [Double#doubleToLongBits] rather than the raw
variants, so a NaN keeps comparing
+ /// equal to a NaN exactly as [Float#equals] does today.
+ @Nullable
+ private static Long toValueWord(DataType storedType, @Nullable
Comparable<?> value) {
+ switch (storedType) {
+ case INT:
Review Comment:
**MINOR (allocation):** `toValueWord` returns a boxed `Long`, so `build()`
allocates up to two throwaway `Long`s per build outside the -128..127 cache.
The Builder Javadoc (`:857-861`) notes this path runs per query per segment for
virtual columns (confirmed: `ImmutableSegmentImpl:479-489` rebuilds the virtual
data source on every call). Negligible, but trivially avoidable by hoisting the
`instanceof` check and returning a primitive.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -74,24 +74,53 @@
/// one instance per distinct spec instead of retaining its own. Callers must
treat shared specs and their nested
/// values as read-only. Deserialize [FieldSpec#toJsonObject()] to make a copy
before editing a spec.
///
-/// The object layout is kept at 72 bytes for an ordinary column for the same
reason: the six booleans and the
-/// forward-index encoding are packed into one [#_flags] byte, and the refs
only a partitioned column or an OPEN_STRUCT
-/// parent/child carries (partition function and partitions, parent column,
sparse keys) live in a lazily allocated
-/// [Extras] holder that stays `null` for every other column. The compression
stats stay a direct ref because the
-/// segment creator writes them for every raw column. None of this is visible
through the public getters, so the
-/// `/tables/{table}/segments/{segment}/metadata` payload (bean-serialized
from the getters) is unchanged.
+/// The object layout is kept at 72 bytes for every column for the same
reason: the six booleans, the forward-index
+/// encoding, the two min/max representation bits and `bitsPerElement` are
packed into one [#_flags] int; the refs
+/// only a partitioned column or an OPEN_STRUCT parent/child carries
(partition function and partitions, parent
+/// column, sparse keys) live in a lazily allocated [Extras] holder that stays
`null` for every other column; and the
+/// three element-length ints share their two words with the numeric min/max
(see [#_minWord]). The compression stats
+/// stay a direct ref because the segment creator writes them for every raw
column. None of this is visible through
+/// the public getters, so the `/tables/{table}/segments/{segment}/metadata`
payload (bean-serialized from the
+/// getters) is unchanged.
+///
+/// | bytes | field(s) |
+/// |------:|----------|
+/// | 12 | object header |
+/// | 16 | `_minWord`, `_maxWord` |
+/// | 16 | `_totalDocs`, `_cardinality`, `_totalNumberOfEntries`,
`_maxNumberOfMultiValues` |
+/// | 4 | `_flags` |
+/// | 24 | `_fieldSpec`, `_minValue`, `_maxValue`, `_extras`,
`_compressionMetadata`, `_indexTypeSizes` |
+/// | 72 | total |
+///
+/// The saving over the eight ints, six refs and flags byte this replaced is
not in the object itself, which is the
+/// same 72 bytes, but in what it no longer retains: a fixed-width column
holds no box per min/max value, which is
+/// ~32 bytes and two surviving objects per numeric column.
@SuppressWarnings({"rawtypes", "unchecked"})
public class ColumnMetadataImpl implements ColumnMetadata {
private static final long SIZE_MASK = 0xffffffffffffL;
// Bits of _flags
- private static final byte HAS_DICTIONARY = 1;
- private static final byte DICTIONARY_ENCODED_FORWARD_INDEX = 1 << 1;
- private static final byte SORTED = 1 << 2;
- private static final byte NON_NULL = 1 << 3;
- private static final byte MIN_MAX_VALUE_INVALID = 1 << 4;
- private static final byte ASCII = 1 << 5;
- private static final byte AUTO_GENERATED = 1 << 6;
+ private static final int HAS_DICTIONARY = 1;
+ private static final int DICTIONARY_ENCODED_FORWARD_INDEX = 1 << 1;
+ private static final int SORTED = 1 << 2;
+ private static final int NON_NULL = 1 << 3;
+ private static final int MIN_MAX_VALUE_INVALID = 1 << 4;
+ private static final int ASCII = 1 << 5;
+ private static final int AUTO_GENERATED = 1 << 6;
+ /// Set when the min (max) value is held as raw bits in [#_minWord]
([#_maxWord]) rather than as an object in
+ /// [#_minValue] ([#_maxValue]); an absent value sets neither.
+ private static final int MIN_VALUE_IN_WORD = 1 << 7;
+ private static final int MAX_VALUE_IN_WORD = 1 << 8;
+
+ // The remaining 23 bits of _flags hold bitsPerElement + 1, so that the
UNAVAILABLE sentinel encodes as 0. The
+ // segment creator writes getNumBitsPerValue(cardinality - 1), which never
exceeds Integer.SIZE, but the value is
+ // read verbatim from metadata.properties and the Builder is public, so a
value outside the encodable range falls
+ // back to Extras rather than being truncated.
Review Comment:
**MINOR (design):** `bitsPerElement` takes all 23 remaining bits of
`_flags`, leaving zero headroom for a future boolean. Every real producer
writes 1..32 or -1 (`BaseSegmentCreator.java:655-656`,
`ForwardIndexHandler.java:1082-1083`/`:1161-1162`,
`DictionaryToRawIndexConverter.java:251`; `PinotDataBitSet.getNumBitsPerValue`
caps at 32). A 7-bit field (-1..125 encodable, else the existing `Extras`
fallback) keeps the same semantics and frees 16 bits. Design choice, not a
defect.
##########
pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImplTest.java:
##########
@@ -354,6 +356,309 @@ public void flagsRoundTripIndependently() {
none.toString());
}
+ /// The min/max value of a fixed-width stored type is held as raw bits and
boxed on read, so every stored type must
+ /// come back as the very value
[ColumnMetadataImpl#fromPropertiesConfiguration] parsed: same class, same value.
+ @Test
+ public void minMaxValuesRoundTripForEveryDataType() {
+ Map<DataType, List<Object>> expected = new LinkedHashMap<>();
+ expected.put(DataType.INT, List.of("-5", "7", -5, 7));
+ expected.put(DataType.LONG, List.of("-5000000000", "7000000000",
-5000000000L, 7000000000L));
+ expected.put(DataType.FLOAT, List.of("-1.5", "2.5", -1.5f, 2.5f));
+ expected.put(DataType.DOUBLE, List.of("-1.5", "2.5", -1.5d, 2.5d));
+ // BOOLEAN is stored as INT and TIMESTAMP as LONG, so their min/max are
the stored type's box.
+ expected.put(DataType.BOOLEAN, List.of("0", "1", 0, 1));
+ expected.put(DataType.TIMESTAMP, List.of("1000", "2000", 1000L, 2000L));
+ expected.put(DataType.STRING, List.of("aa", "zz", "aa", "zz"));
+ expected.put(DataType.JSON, List.of("{}", "{}", "{}", "{}"));
+ expected.put(DataType.BIG_DECIMAL, List.of("-1.50", "2.5", new
BigDecimal("-1.50"), new BigDecimal("2.5")));
+
+ expected.forEach((dataType, values) -> {
+ ColumnMetadataImpl metadata = withMinMax(dataType, (String)
values.get(0), (String) values.get(1));
+ assertEquals(metadata.getMinValue(), values.get(2), dataType.name());
+ assertEquals(metadata.getMaxValue(), values.get(3), dataType.name());
+ assertEquals(metadata.getMinValue().getClass(),
values.get(2).getClass(), dataType.name());
+ assertEquals(metadata.getMaxValue().getClass(),
values.get(3).getClass(), dataType.name());
+ assertFalse(metadata.isMinMaxValueInvalid(), dataType.name());
+ // The REST payload is serialized from the getters, so it carries
exactly the node the value serializes to.
+ JsonNode json = JsonUtils.objectToJsonNode(metadata);
+ assertEquals(json.get("minValue"),
JsonUtils.objectToJsonNode(values.get(2)), dataType.name());
+ assertEquals(json.get("maxValue"),
JsonUtils.objectToJsonNode(values.get(3)), dataType.name());
+ });
+
+ // BYTES parses to a ByteArray, which is var-width and therefore always
kept as an object.
+ ColumnMetadataImpl bytes = withMinMax(DataType.BYTES, "0a0b", "ff");
+ assertEquals(bytes.getMinValue(), BytesUtils.toByteArray("0a0b"));
+ assertEquals(bytes.getMaxValue(), BytesUtils.toByteArray("ff"));
+ }
+
+ /// A column with no min/max keeps reporting `null` for both, and the
min-max-invalid flag is independent of them.
+ @Test
+ public void minMaxValuesAbsentOrInvalid() {
+ for (DataType dataType : List.of(DataType.INT, DataType.LONG,
DataType.FLOAT, DataType.DOUBLE, DataType.BOOLEAN,
+ DataType.TIMESTAMP, DataType.STRING, DataType.BYTES,
DataType.BIG_DECIMAL)) {
+ ColumnMetadataImpl absent = withMinMax(dataType, null, null);
+ assertNull(absent.getMinValue(), dataType.name());
+ assertNull(absent.getMaxValue(), dataType.name());
+ assertFalse(absent.isMinMaxValueInvalid(), dataType.name());
+
+ PropertiesConfiguration invalidConfig = minMaxConfig(dataType, null,
null);
+ invalidConfig.setProperty(Column.getKeyFor("col",
Column.MIN_MAX_VALUE_INVALID), true);
+ ColumnMetadataImpl invalid =
ColumnMetadataImpl.fromPropertiesConfiguration(invalidConfig, 10, "col");
+ assertNull(invalid.getMinValue(), dataType.name());
+ assertNull(invalid.getMaxValue(), dataType.name());
+ assertTrue(invalid.isMinMaxValueInvalid(), dataType.name());
+ assertNotEquals(invalid, absent, dataType.name());
+
+ // Only one of the two present: the other stays null rather than reading
back the packed zero.
+ String value = dataType == DataType.BYTES ? "0a" : "1";
+ ColumnMetadataImpl minOnly = withMinMax(dataType, value, null);
+ assertNotNull(minOnly.getMinValue(), dataType.name());
+ assertNull(minOnly.getMaxValue(), dataType.name());
+ ColumnMetadataImpl maxOnly = withMinMax(dataType, null, value);
+ assertNull(maxOnly.getMinValue(), dataType.name());
+ assertNotNull(maxOnly.getMaxValue(), dataType.name());
+ assertNotEquals(maxOnly, minOnly, dataType.name());
+ }
+
+ // A COMPLEX column has no min/max at all and is flagged invalid.
+ PropertiesConfiguration complexConfig = complexConfig("metrics", "cpu");
+ complexConfig.setProperty(Column.getKeyFor("metrics", Column.CARDINALITY),
1);
+ ColumnMetadataImpl complex =
ColumnMetadataImpl.fromPropertiesConfiguration(complexConfig, 10, "metrics");
+ assertNull(complex.getMinValue());
+ assertNull(complex.getMaxValue());
+ assertTrue(complex.isMinMaxValueInvalid());
+ }
+
+ /// A packed min/max takes part in equality, hashCode and toString exactly
as the boxed value did, including the
+ /// FLOAT/DOUBLE corner cases where bit equality and [Float#equals] must
agree.
+ @Test
+ public void packedMinMaxValuesParticipateInValueObjectMethods() {
+ ColumnMetadataImpl first = withMinMax(DataType.INT, "-5", "7");
+ assertEquals(first, withMinMax(DataType.INT, "-5", "7"));
+ assertEquals(first.hashCode(), withMinMax(DataType.INT, "-5",
"7").hashCode());
+ assertNotEquals(first, withMinMax(DataType.INT, "-5", "8"));
+ assertNotEquals(first, withMinMax(DataType.INT, "-5", null));
+ assertTrue(first.toString().contains("_minValue=-5, _maxValue=7"),
first.toString());
+
+ // Zero is the value the words hold when a value is absent, so it must
stay distinguishable from absence.
+ ColumnMetadataImpl zero = withMinMax(DataType.INT, "0", "0");
+ assertEquals(zero.getMinValue(), 0);
+ assertNotEquals(zero, withMinMax(DataType.INT, null, null));
+
+ // -0.0 is not equal to 0.0 for Float/Double, and NaN is equal to itself:
bit equality agrees with both.
+ assertNotEquals(withMinMax(DataType.FLOAT, "-0.0", "1").getMinValue(),
+ withMinMax(DataType.FLOAT, "0.0", "1").getMinValue());
+ assertNotEquals(withMinMax(DataType.DOUBLE, "-0.0", "1"),
withMinMax(DataType.DOUBLE, "0.0", "1"));
+ ColumnMetadataImpl nan = withMinMax(DataType.DOUBLE, "NaN", "NaN");
+ assertEquals(nan.getMinValue(), Double.NaN);
+ assertEquals(nan, withMinMax(DataType.DOUBLE, "NaN", "NaN"));
+ assertEquals(nan.hashCode(), withMinMax(DataType.DOUBLE, "NaN",
"NaN").hashCode());
+ }
+
+ /// [ColumnMetadataImpl.Builder] is public, so a caller may hand a
fixed-width column a min/max that is not the box
+ /// class of its stored type. Such a value cannot be packed and must survive
as the object it is.
+ @Test
+ public void minMaxOfUnexpectedTypeIsKeptVerbatim() {
+ ColumnMetadataImpl metadata = unexpectedMinMax();
+ assertEquals(metadata.getMinValue(), "not-an-int");
+ assertEquals(metadata.getMaxValue(), 3L);
+ assertEquals(metadata, unexpectedMinMax());
+ assertEquals(metadata.hashCode(), unexpectedMinMax().hashCode());
+ // The lengths of a fixed-width column are derived, so the fallback cannot
disturb them.
+ assertEquals(metadata.getLengthOfLongestElement(), Integer.BYTES);
+ }
+
+ private static ColumnMetadataImpl unexpectedMinMax() {
+ return ColumnMetadataImpl.builder()
+ .setFieldSpec(new DimensionFieldSpec("col", DataType.INT, true))
+ .setTotalDocs(10)
+ .setMinValue("not-an-int")
+ .setMaxValue(3L)
+ .build();
+ }
+
+ /// The element lengths share their two words with the numeric min/max, so a
var-width column must round-trip all
+ /// three of them while a fixed-width column derives them from its stored
type and its multi-value count.
+ @Test
+ public void elementLengthsRoundTrip() {
+ ColumnMetadataImpl varWidthMv = ColumnMetadataImpl.builder()
+ .setFieldSpec(new DimensionFieldSpec("col", DataType.STRING, false))
+ .setTotalDocs(10)
+ .setLengthOfShortestElement(2)
+ .setLengthOfLongestElement(7)
+ .setMaxNumberOfMultiValues(3)
+ .setMaxRowLengthInBytes(15)
+ .setTotalNumberOfEntries(30)
+ .setMinValue("aa")
+ .setMaxValue("zz")
+ .build();
+ assertEquals(varWidthMv.getLengthOfShortestElement(), 2);
+ assertEquals(varWidthMv.getLengthOfLongestElement(), 7);
+ assertEquals(varWidthMv.getMaxRowLengthInBytes(), 15);
+ assertEquals(varWidthMv.getMaxNumberOfMultiValues(), 3);
+ assertEquals(varWidthMv.getTotalNumberOfEntries(), 30);
+ assertEquals(varWidthMv.getMinValue(), "aa");
+ assertEquals(varWidthMv.getMaxValue(), "zz");
+ assertFalse(varWidthMv.isFixedLength());
+
+ // A var-width SV column: the max row length is the longest element.
+ ColumnMetadataImpl varWidthSv = ColumnMetadataImpl.builder()
+ .setFieldSpec(new DimensionFieldSpec("col", DataType.STRING, true))
+
.setTotalDocs(10).setLengthOfShortestElement(4).setLengthOfLongestElement(4).build();
+ assertEquals(varWidthSv.getMaxRowLengthInBytes(), 4);
+ assertEquals(varWidthSv.getTotalNumberOfEntries(), 10);
+ assertEquals(varWidthSv.getMaxNumberOfMultiValues(), 0);
+ assertTrue(varWidthSv.isFixedLength());
+
+ // Pre-1.6.0 raw var-width columns write no lengths at all: the
UNAVAILABLE sentinel must survive the packing.
+ ColumnMetadataImpl unavailable =
ColumnMetadataImpl.fromPropertiesConfiguration(baseConfig("col"), 10, "col");
+ assertEquals(unavailable.getLengthOfShortestElement(),
ColumnMetadata.UNAVAILABLE);
+ assertEquals(unavailable.getLengthOfLongestElement(),
ColumnMetadata.UNAVAILABLE);
+ assertEquals(unavailable.getMaxRowLengthInBytes(),
ColumnMetadata.UNAVAILABLE);
+
+ // Fixed-width columns derive all three from the stored type, min/max
being packed in the same words.
+ ColumnMetadataImpl fixedSv = ColumnMetadataImpl.builder()
+ .setFieldSpec(new DimensionFieldSpec("col", DataType.TIMESTAMP, true))
+ .setTotalDocs(10).setMinValue(1L).setMaxValue(2L).build();
+ assertEquals(fixedSv.getLengthOfShortestElement(), Long.BYTES);
+ assertEquals(fixedSv.getLengthOfLongestElement(), Long.BYTES);
+ assertEquals(fixedSv.getMaxRowLengthInBytes(), Long.BYTES);
+ assertEquals(fixedSv.getMinValue(), 1L);
+
+ ColumnMetadataImpl fixedMv = ColumnMetadataImpl.builder()
+ .setFieldSpec(new DimensionFieldSpec("col", DataType.INT, false))
+
.setTotalDocs(10).setMaxNumberOfMultiValues(3).setTotalNumberOfEntries(25).setMinValue(1).setMaxValue(2)
+ .build();
+ assertEquals(fixedMv.getLengthOfLongestElement(), Integer.BYTES);
+ assertEquals(fixedMv.getMaxRowLengthInBytes(), 3 * Integer.BYTES);
+ assertEquals(fixedMv.getTotalNumberOfEntries(), 25);
+ assertEquals(fixedMv.getMinValue(), 1);
+ }
+
+ /// `bitsPerElement` is packed into the flags word rather than held in its
own int, so every value a segment or a
+ /// [ColumnMetadataImpl.Builder] caller can produce must come back verbatim
- including the ones too large to
+ /// encode, which fall back to the [ColumnMetadataImpl] extras holder.
+ @Test
+ public void bitsPerElementRoundTrips() {
+ // -1 is the UNAVAILABLE sentinel of a raw column, 1..32 is what the
segment creator writes, and the rest are
+ // values only a hand-written or corrupt metadata.properties can carry.
+ for (int bitsPerElement : new int[]{
+ ColumnMetadata.UNAVAILABLE, 0, 1, 8, 32, 8388604, 8388605, 8388606,
Integer.MAX_VALUE, -2, Integer.MIN_VALUE
+ }) {
+ ColumnMetadataImpl metadata = withBitsPerElement(bitsPerElement);
+ String message = "bitsPerElement " + bitsPerElement;
+ assertEquals(metadata.getBitsPerElement(), bitsPerElement, message);
+ assertEquals(metadata, withBitsPerElement(bitsPerElement), message);
+ assertEquals(metadata.hashCode(),
withBitsPerElement(bitsPerElement).hashCode(), message);
+ assertNotEquals(metadata, withBitsPerElement(bitsPerElement - 1),
message);
+
assertEquals(JsonUtils.objectToJsonNode(metadata).get("bitsPerElement").asInt(),
bitsPerElement, message);
+ assertTrue(metadata.toString().contains("_bitsPerElement=" +
bitsPerElement), metadata.toString());
+ // The packing shares its word with the flags, so neither may bleed into
the other.
+ assertTrue(metadata.hasDictionary(), message);
+ assertTrue(metadata.isSorted(), message);
+ assertTrue(metadata.isNonNull(), message);
+ assertTrue(metadata.isAutoGenerated(), message);
+ assertTrue(metadata.isAscii(), message);
+ assertTrue(metadata.isMinMaxValueInvalid(), message);
+ assertEquals(metadata.getForwardIndexEncoding(),
EncodingType.DICTIONARY, message);
+ }
+
+ // An unencodable value shares the extras holder with the rare refs, so
the two must not displace each other.
+ ColumnMetadataImpl withExtras = ColumnMetadataImpl.builder()
+ .setFieldSpec(new DimensionFieldSpec("col", DataType.INT, true))
+
.setTotalDocs(10).setHasDictionary(true).setBitsPerElement(Integer.MAX_VALUE).setParentColumn("parent")
+ .build();
+ assertEquals(withExtras.getBitsPerElement(), Integer.MAX_VALUE);
+ assertEquals(withExtras.getParentColumn(), "parent");
+ assertNotEquals(withExtras, ColumnMetadataImpl.builder()
+ .setFieldSpec(new DimensionFieldSpec("col", DataType.INT, true))
+
.setTotalDocs(10).setHasDictionary(true).setBitsPerElement(Integer.MAX_VALUE -
1).setParentColumn("parent")
+ .build());
+ }
+
+ /// The four ints that describe the shape of a column are read back from the
instance itself and each of them
+ /// distinguishes two otherwise identical columns.
Review Comment:
**MINOR (naming):** `columnShapeFieldsRoundTrip` and "the four ints that
describe the shape of a column" are the vocabulary of the dropped `SharedShape`
design from the first commit — the only residue left (no `SharedShape` /
`SHAPE_INTERNER` / `_shape` survives in the head tree). Consider renaming so a
future reader does not go looking for a shape object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
