diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java index d4fa8d606188..08564230f148 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java @@ -19,16 +19,23 @@ package org.apache.pinot.segment.local.segment.index; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.io.FileUtils; import org.apache.pinot.segment.local.segment.creator.SegmentTestUtils; import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl; import org.apache.pinot.segment.local.segment.index.converter.SegmentV1V2ToV3FormatConverter; +import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader; import org.apache.pinot.segment.spi.ColumnMetadata; import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig; import org.apache.pinot.segment.spi.creator.SegmentIndexCreationDriver; @@ -37,6 +44,18 @@ import org.apache.pinot.segment.spi.index.metadata.ColumnMetadataImpl; import org.apache.pinot.segment.spi.index.metadata.SegmentMetadataImpl; import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths; +import org.apache.pinot.spi.config.table.FieldConfig; +import org.apache.pinot.spi.config.table.OpenStructIndexConfig; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.TableType; +import org.apache.pinot.spi.data.ComplexFieldSpec; +import org.apache.pinot.spi.data.DimensionFieldSpec; +import org.apache.pinot.spi.data.FieldSpec; +import org.apache.pinot.spi.data.OpenStructNaming; +import org.apache.pinot.spi.data.Schema; +import org.apache.pinot.spi.data.readers.GenericRow; +import org.apache.pinot.spi.utils.JsonUtils; +import org.apache.pinot.spi.utils.builder.TableConfigBuilder; import org.apache.pinot.util.TestUtils; import org.testng.Assert; import org.testng.annotations.AfterMethod; @@ -44,11 +63,13 @@ import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertSame; public class SegmentMetadataImplTest { private static final String AVRO_DATA = "data/test_data-mv.avro"; private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "SegmentMetadataImplTest"); + private File _avroFile; private File _segmentDirectory; @BeforeMethod @@ -56,10 +77,11 @@ public void setUp() throws Exception { final String filePath = TestUtils.getFileFromResourceUrl(SegmentMetadataImplTest.class.getClassLoader().getResource(AVRO_DATA)); + _avroFile = new File(filePath); // intentionally changed this to TimeUnit.Hours to make it non-default for testing final SegmentGeneratorConfig config = SegmentTestUtils - .getSegmentGenSpecWithSchemAndProjectedColumns(new File(filePath), INDEX_DIR, "daysSinceEpoch", TimeUnit.HOURS, + .getSegmentGenSpecWithSchemAndProjectedColumns(_avroFile, INDEX_DIR, "daysSinceEpoch", TimeUnit.HOURS, "testTable"); config.setSegmentNamePostfix("1"); config.setCustomProperties(Map.of("custom.k1", "v1", "custom.k2", "v2")); @@ -141,4 +163,104 @@ public void testIndexSizesOnlyFromIndexDir() Assert.assertTrue(((ColumnMetadataImpl) streamColumn).getIndexSizeMap().isEmpty(), entry.getKey()); } } + + /// A server holds one column-metadata graph per loaded segment, so the per-column strings and default null values + /// are shared: two loads of the same metadata alias one interned column name (as the map key, the FieldSpec name and + /// the Schema entry) and hold the static FieldSpec default constant rather than a box parsed from the literal the + /// segment creator wrote, while the specs stay equal to the schema the segment was built from. + @Test + public void testColumnStringsAndDefaultsSharedAcrossLoads() + throws Exception { + SegmentMetadataImpl first = new SegmentMetadataImpl(_segmentDirectory); + SegmentMetadataImpl second = new SegmentMetadataImpl(_segmentDirectory); + assertEquals(first.getColumnMetadataMap().keySet(), second.getColumnMetadataMap().keySet()); + assertSame(first.getColumnMetadataMap().firstKey(), second.getColumnMetadataMap().firstKey()); + assertSame(first.getSchema().getDimensionNames().get(0), second.getSchema().getDimensionNames().get(0)); + Iterator secondKeys = second.getColumnMetadataMap().keySet().iterator(); + for (String column : first.getColumnMetadataMap().keySet()) { + assertSame(column, secondKeys.next()); + FieldSpec fieldSpec = first.getColumnMetadataFor(column).getFieldSpec(); + assertSame(fieldSpec.getName(), column, column); + assertSame(fieldSpec.getName(), second.getColumnMetadataFor(column).getFieldSpec().getName(), column); + assertSame(first.getSchema().getFieldSpecFor(column).getName(), column, column); + assertSame(fieldSpec.getDefaultNullValue(), + FieldSpec.getDefaultNullValue(fieldSpec.getFieldType(), fieldSpec.getDataType(), null), column); + } + Schema inputSchema = SegmentTestUtils.extractSchemaFromAvroWithoutTime(_avroFile); + for (FieldSpec inputFieldSpec : inputSchema.getAllFieldSpecs()) { + assertEquals(first.getSchema().getFieldSpecFor(inputFieldSpec.getName()), inputFieldSpec); + } + } + + /// OPEN_STRUCT children carry an explicit column name and a parent column in `metadata.properties`; both come back + /// as the interned instances, so the child's parent name is the very String that keys the parent column. + @Test + public void testOpenStructChildStringsShared() + throws Exception { + String parent = "metrics"; + File segmentDir = buildOpenStructSegment(parent); + try { + SegmentMetadataImpl first = new SegmentMetadataImpl(segmentDir); + SegmentMetadataImpl second = new SegmentMetadataImpl(segmentDir); + String parentKey = first.getColumnMetadataMap().ceilingKey(parent); + assertEquals(parentKey, parent); + assertSame(parentKey, second.getColumnMetadataMap().ceilingKey(parent)); + String child = OpenStructNaming.materializedColumnName(parent, "cpu"); + ColumnMetadataImpl firstChild = (ColumnMetadataImpl) first.getColumnMetadataFor(child); + ColumnMetadataImpl secondChild = (ColumnMetadataImpl) second.getColumnMetadataFor(child); + assertEquals(firstChild.getParentColumn(), parent); + assertSame(firstChild.getParentColumn(), parentKey); + assertSame(firstChild.getParentColumn(), secondChild.getParentColumn()); + assertSame(firstChild.getFieldSpec().getName(), secondChild.getFieldSpec().getName()); + assertSame(firstChild.getFieldSpec().getDefaultNullValue(), FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_DOUBLE); + ComplexFieldSpec firstParent = (ComplexFieldSpec) first.getColumnMetadataFor(parent).getFieldSpec(); + ComplexFieldSpec secondParent = (ComplexFieldSpec) second.getColumnMetadataFor(parent).getFieldSpec(); + assertEquals(firstParent.getChildFieldSpecs().keySet(), Set.of("views", "cpu", "host")); + for (Map.Entry entry : firstParent.getChildFieldSpecs().entrySet()) { + FieldSpec childSpec = entry.getValue(); + assertSame(childSpec.getName(), entry.getKey()); + assertSame(childSpec.getName(), secondParent.getChildFieldSpec(entry.getKey()).getName()); + assertSame(childSpec.getDefaultNullValue(), + FieldSpec.getDefaultNullValue(childSpec.getFieldType(), childSpec.getDataType(), null)); + } + } finally { + FileUtils.deleteQuietly(segmentDir); + } + } + + private static File buildOpenStructSegment(String parent) + throws Exception { + Map children = new HashMap<>(); + children.put("views", new DimensionFieldSpec("views", FieldSpec.DataType.LONG, true)); + children.put("cpu", new DimensionFieldSpec("cpu", FieldSpec.DataType.DOUBLE, true)); + children.put("host", new DimensionFieldSpec("host", FieldSpec.DataType.STRING, true)); + Schema schema = new Schema.SchemaBuilder().setSchemaName("testOpenStruct") + .addField(new ComplexFieldSpec(parent, FieldSpec.DataType.OPEN_STRUCT, true, children)) + .addSingleValueDimension("dim", FieldSpec.DataType.STRING) + .build(); + OpenStructIndexConfig openStructConfig = + new OpenStructIndexConfig(false, null, 3, Set.of("views", "cpu", "host"), 0.5, List.of(), null); + ObjectNode indexes = JsonUtils.newObjectNode(); + indexes.set("open_struct", JsonUtils.objectToJsonNode(openStructConfig)); + TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName("testOpenStruct") + .setFieldConfigList(List.of(new FieldConfig.Builder(parent).withIndexes(indexes).build())).build(); + SegmentGeneratorConfig config = new SegmentGeneratorConfig(tableConfig, schema); + config.setOutDir(new File(INDEX_DIR, "openStruct").getAbsolutePath()); + config.setSegmentName("openStructSegment"); + List rows = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + GenericRow row = new GenericRow(); + Map metrics = new HashMap<>(); + metrics.put("views", (long) i); + metrics.put("cpu", i * 0.5); + metrics.put("host", "host-" + i); + row.putValue(parent, metrics); + row.putValue("dim", "val-" + i); + rows.add(row); + } + SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl(); + driver.init(config, new GenericRowRecordReader(rows)); + driver.build(); + return driver.getOutputDirectory(); + } } diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java index d79515ba9485..8ed3b65f2208 100644 --- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java +++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java @@ -19,6 +19,7 @@ package org.apache.pinot.segment.spi.index.metadata; import com.fasterxml.jackson.core.type.TypeReference; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; import it.unimi.dsi.fastutil.ints.IntSet; import java.math.BigDecimal; @@ -57,6 +58,14 @@ import org.apache.pinot.spi.utils.JsonUtils; +/// Column metadata parsed from `metadata.properties` (or built through [Builder]). +/// +/// A server retains one instance per (segment, column) for as long as the segment is loaded, so the parse path keeps +/// the per-column footprint small: column names, parent-column names, date-time formats/granularities and custom +/// default-null literals are interned (they recur in every segment of a table), and a `defaultNullValue` that equals +/// the type default is not handed to the [FieldSpec] at all, so the spec carries the shared static +/// `FieldSpec.DEFAULT_*` constant and never retains the literal. Segment-derived [FieldSpec]s must therefore be +/// treated as read-only: nothing may mutate their default null value in place (nothing ever did). @SuppressWarnings({"rawtypes", "unchecked"}) public class ColumnMetadataImpl implements ColumnMetadata { private static final long SIZE_MASK = 0xffffffffffffL; @@ -419,7 +428,7 @@ public static ColumnMetadataImpl fromPropertiesConfiguration(PropertiesConfigura .setMaxRowLengthInBytes(config.getInt(Column.getKeyFor(column, Column.MAX_ROW_LENGTH_IN_BYTES), UNAVAILABLE)) .setBitsPerElement(config.getInt(Column.getKeyFor(column, Column.BITS_PER_ELEMENT), UNAVAILABLE)) .setAutoGenerated(config.getBoolean(Column.getKeyFor(column, Column.IS_AUTO_GENERATED), false)) - .setParentColumn(config.getString(Column.getKeyFor(column, Column.PARENT_COLUMN), null)); + .setParentColumn(intern(config.getString(Column.getKeyFor(column, Column.PARENT_COLUMN), null))); Object rawSparseKeys = config.getProperty(Column.getKeyFor(column, Column.SPARSE_KEYS)); if (rawSparseKeys != null) { @@ -496,7 +505,11 @@ private static ChunkCompressionType parseCompressionType(String column, @Nullabl } public static FieldSpec extractFieldSpec(String column, PropertiesConfiguration config) { - String fieldName = config.getString(Column.getKeyFor(column, Column.COLUMN_NAME), column); + // The name is retained by the FieldSpec, the segment Schema and every per-segment column map, and it recurs in + // every segment of the table: intern it so all of them alias one JVM-wide instance. When COLUMN_NAME is absent + // (the segment creator only writes it when it differs from the key) this is the key parsed by SegmentMetadataImpl, + // which is already interned, so the lookup just returns it. + String fieldName = config.getString(Column.getKeyFor(column, Column.COLUMN_NAME), column).intern(); FieldType fieldType = config.getEnum(Column.getKeyFor(column, Column.COLUMN_TYPE), FieldType.class); DataType dataType = config.getEnum(Column.getKeyFor(column, Column.DATA_TYPE), DataType.class); boolean isSingleValue = config.getBoolean(Column.getKeyFor(column, Column.IS_SINGLE_VALUED), true); @@ -511,24 +524,26 @@ public static FieldSpec extractFieldSpec(String column, PropertiesConfiguration ? FieldSpec.MaxLengthExceedStrategy.valueOf(maxLengthExceedStrategyString) : null; switch (fieldType) { case DIMENSION: - return new DimensionFieldSpec(fieldName, dataType, isSingleValue, maxLength, defaultNullValueString, - maxLengthExceedStrategy); + return new DimensionFieldSpec(fieldName, dataType, isSingleValue, maxLength, + canonicalDefaultNullValue(fieldType, dataType, defaultNullValueString), maxLengthExceedStrategy); case METRIC: - return new MetricFieldSpec(fieldName, dataType, defaultNullValueString, maxLength, maxLengthExceedStrategy); + return new MetricFieldSpec(fieldName, dataType, + canonicalDefaultNullValue(fieldType, dataType, defaultNullValueString), maxLength, maxLengthExceedStrategy); case TIME: TimeUnit timeUnit = TimeUnit.valueOf(config.getString(Segment.TIME_UNIT, "DAYS").toUpperCase()); return new TimeFieldSpec(new TimeGranularitySpec(dataType, timeUnit, fieldName)); case DATE_TIME: - String format = config.getString(Column.getKeyFor(column, Column.DATETIME_FORMAT)); - String granularity = config.getString(Column.getKeyFor(column, Column.DATETIME_GRANULARITY)); - return new DateTimeFieldSpec(fieldName, dataType, format, granularity, defaultNullValueString, null); + String format = intern(config.getString(Column.getKeyFor(column, Column.DATETIME_FORMAT))); + String granularity = intern(config.getString(Column.getKeyFor(column, Column.DATETIME_GRANULARITY))); + return new DateTimeFieldSpec(fieldName, dataType, format, granularity, + canonicalDefaultNullValue(fieldType, dataType, defaultNullValueString), null); case COMPLEX: List childFieldNames = config.getList(String.class, Column.getKeyFor(column, Column.COMPLEX_CHILD_FIELD_NAMES)); Map childFieldSpecs = new HashMap<>(); if (childFieldNames != null) { for (String childField : childFieldNames) { - childFieldSpecs.put(childField, + childFieldSpecs.put(childField.intern(), extractFieldSpec(ComplexFieldSpec.getFullChildName(column, childField), config)); } } @@ -538,6 +553,37 @@ public static FieldSpec extractFieldSpec(String column, PropertiesConfiguration } } + /// Returns the `defaultNullValue` literal to hand to the [FieldSpec] constructor: `null` when the literal parses to + /// the type default, so the spec ends up holding the shared static `FieldSpec.DEFAULT_*` constant instead of a + /// per-segment box plus the literal (the segment creator writes the literal for every column, so without this every + /// column of every segment paid for it); otherwise the interned literal, so a custom default is shared across the + /// segments of the table. Equality is [DataType#equals(Object, Object)], the predicate [FieldSpec#equals] applies to + /// default null values, so the canonical spec equals one built from the literal and + /// [FieldSpec#getDefaultNullValueString()] (derived from the value) is unchanged; a BIG_DECIMAL literal with a + /// different scale or a negative-zero FLOAT/DOUBLE is not equal and stays verbatim. + @VisibleForTesting + @Nullable + static String canonicalDefaultNullValue(FieldType fieldType, DataType dataType, @Nullable String literal) { + if (literal == null) { + return null; + } + Object typeDefault; + try { + typeDefault = FieldSpec.getDefaultNullValue(fieldType, dataType, null); + } catch (IllegalStateException e) { + // No type default for this combination (e.g. a METRIC BOOLEAN): the literal is the only valid value, exactly as + // the FieldSpec constructor treats it. + return literal.intern(); + } + return dataType.equals(FieldSpec.getDefaultNullValue(fieldType, dataType, literal), typeDefault) ? null + : literal.intern(); + } + + @Nullable + private static String intern(@Nullable String value) { + return value != null ? value.intern() : null; + } + @Nullable public static PartitionFunction extractPartitionFunction(String column, PropertiesConfiguration config) { String partitionFunctionName = config.getString(Column.getKeyFor(column, Column.PARTITION_FUNCTION), null); diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java index 11e4ffd7c29d..b57434107b45 100644 --- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java +++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java @@ -311,9 +311,14 @@ private static void setCustomConfigs(Configuration segmentMetadataPropertiesConf } /// Helper method to add the physical columns from source list to destination set. + /// + /// Column names are interned: the same names recur in every segment of a table and each one is retained by the + /// column metadata map key, the FieldSpec, the segment Schema and the loader's per-column maps, so one JVM-wide + /// instance replaces a copy per segment (the JVM string table holds them weakly, so they live exactly as long as a + /// loaded segment references them). private static void addPhysicalColumns(List src, Set dest) { for (Object o : src) { - String column = o.toString(); + String column = o.toString().intern(); if (!column.isEmpty() && !BuiltInVirtualColumn.BUILT_IN_VIRTUAL_COLUMNS.contains(column)) { // NOTE: // Exclude built in virtual columns. In regular case they shouldn't exist in the metadata file, but we perform diff --git a/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImplTest.java b/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImplTest.java index 910e7080db71..44913ad14e95 100644 --- a/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImplTest.java +++ b/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImplTest.java @@ -19,21 +19,32 @@ package org.apache.pinot.segment.spi.index.metadata; import com.fasterxml.jackson.databind.JsonNode; +import java.math.BigDecimal; +import java.util.Map; +import javax.annotation.Nullable; import org.apache.commons.configuration2.PropertiesConfiguration; import org.apache.pinot.segment.spi.ColumnMetadata; import org.apache.pinot.segment.spi.V1Constants.MetadataKeys.Column; import org.apache.pinot.segment.spi.compression.ChunkCompressionType; import org.apache.pinot.spi.config.table.FieldConfig.EncodingType; +import org.apache.pinot.spi.data.DateTimeFieldSpec; import org.apache.pinot.spi.data.DimensionFieldSpec; +import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.data.FieldSpec.FieldType; +import org.apache.pinot.spi.data.MetricFieldSpec; +import org.apache.pinot.spi.env.CommonsConfigurationUtils; +import org.apache.pinot.spi.utils.BytesUtils; import org.apache.pinot.spi.utils.JsonUtils; +import org.apache.pinot.spi.utils.UuidUtils; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertNotSame; import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertSame; import static org.testng.Assert.assertTrue; import static org.testng.Assert.expectThrows; @@ -266,9 +277,204 @@ public void rejectsInvalidIndexSize() { assertEquals(metadata.getNumIndexes(), 0, "a rejected size must not be recorded"); } + private static final String DATETIME_FORMAT = "1:MILLISECONDS:EPOCH"; + private static final String DATETIME_GRANULARITY = "1:MILLISECONDS"; + + /// The segment creator writes `defaultNullValue` for every column, so a column whose default is the type default + /// must come back holding the shared static constant (one instance per JVM instead of a box plus the literal per + /// segment) while staying equal to the spec the table schema would build. + @Test + public void typeDefaultLiteralSharesTheStaticConstant() { + Map dimensionDefaults = Map.ofEntries( + Map.entry(DataType.INT, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_INT), + Map.entry(DataType.LONG, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_LONG), + Map.entry(DataType.FLOAT, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_FLOAT), + Map.entry(DataType.DOUBLE, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_DOUBLE), + Map.entry(DataType.BOOLEAN, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_BOOLEAN), + Map.entry(DataType.TIMESTAMP, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_TIMESTAMP), + Map.entry(DataType.STRING, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_STRING), + Map.entry(DataType.JSON, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_JSON), + Map.entry(DataType.BYTES, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_BYTES), + Map.entry(DataType.BIG_DECIMAL, FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_BIG_DECIMAL)); + dimensionDefaults.forEach((dataType, constant) -> { + FieldSpec spec = parse(FieldType.DIMENSION, dataType, writtenLiteral(dataType, constant)); + assertSame(spec.getDefaultNullValue(), constant, dataType.name()); + assertEquals(spec.getDefaultNullValueString(), dataType.toString(constant), dataType.name()); + assertEquals(spec, new DimensionFieldSpec("col", dataType, true), dataType.name()); + }); + + Map metricDefaults = Map.of( + DataType.INT, FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_INT, + DataType.LONG, FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_LONG, + DataType.FLOAT, FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_FLOAT, + DataType.DOUBLE, FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_DOUBLE, + DataType.BIG_DECIMAL, FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_BIG_DECIMAL, + DataType.STRING, FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_STRING, + DataType.BYTES, FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_BYTES); + metricDefaults.forEach((dataType, constant) -> { + FieldSpec spec = parse(FieldType.METRIC, dataType, writtenLiteral(dataType, constant)); + assertSame(spec.getDefaultNullValue(), constant, dataType.name()); + assertEquals(spec.getDefaultNullValueString(), dataType.toString(constant), dataType.name()); + assertEquals(spec, new MetricFieldSpec("col", dataType), dataType.name()); + }); + } + + /// The UUID default is a fresh nil-UUID array per lookup, so there is no constant to share; the literal is still + /// recognised as the type default (it is dropped rather than retained) and the value stays equal. + @Test + public void uuidTypeDefaultStaysValueEqual() { + String literal = UuidUtils.toString(UuidUtils.nullUuidBytes()); + assertNull(ColumnMetadataImpl.canonicalDefaultNullValue(FieldType.DIMENSION, DataType.UUID, literal)); + FieldSpec spec = parse(FieldType.DIMENSION, DataType.UUID, literal); + assertEquals((byte[]) spec.getDefaultNullValue(), UuidUtils.nullUuidBytes()); + assertEquals(spec.getDefaultNullValueString(), literal); + assertEquals(spec, new DimensionFieldSpec("col", DataType.UUID, true)); + } + + /// Custom defaults are parsed exactly as before and equal the spec a table schema builds; the literal itself is + /// interned so the segments of a table share it. + @Test + public void customLiteralsRoundTrip() { + FieldSpec intSpec = parse(FieldType.DIMENSION, DataType.INT, "-1"); + assertEquals(intSpec, new DimensionFieldSpec("col", DataType.INT, true, -1)); + assertNotSame(intSpec.getDefaultNullValue(), FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_INT); + assertEquals(parse(FieldType.DIMENSION, DataType.STRING, "N/A"), + new DimensionFieldSpec("col", DataType.STRING, true, "N/A")); + assertEquals(parse(FieldType.DIMENSION, DataType.BYTES, "abcd"), + new DimensionFieldSpec("col", DataType.BYTES, true, BytesUtils.toBytes("abcd"))); + assertEquals(parse(FieldType.METRIC, DataType.DOUBLE, "1.5"), new MetricFieldSpec("col", DataType.DOUBLE, 1.5)); + // Equality is the data type's own: a BIG_DECIMAL with another scale and a negative zero are not the type default, + // so their string form survives the round trip. + FieldSpec scaledZero = parse(FieldType.DIMENSION, DataType.BIG_DECIMAL, "0.0"); + assertNotSame(scaledZero.getDefaultNullValue(), FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_BIG_DECIMAL); + assertEquals(scaledZero.getDefaultNullValueString(), "0.0"); + assertEquals(scaledZero, new DimensionFieldSpec("col", DataType.BIG_DECIMAL, true, new BigDecimal("0.0"))); + FieldSpec negativeZero = parse(FieldType.METRIC, DataType.FLOAT, "-0.0"); + assertNotSame(negativeZero.getDefaultNullValue(), FieldSpec.DEFAULT_METRIC_NULL_VALUE_OF_FLOAT); + assertEquals(negativeZero.getDefaultNullValueString(), "-0.0"); + + assertSame(ColumnMetadataImpl.canonicalDefaultNullValue(FieldType.DIMENSION, DataType.INT, new String("-1")), + "-1"); + assertNull(ColumnMetadataImpl.canonicalDefaultNullValue(FieldType.DIMENSION, DataType.INT, null)); + } + + /// A STRING default with a leading/trailing space or a comma is escaped by the segment creator and recovered here + /// before the type-default comparison, so it round-trips verbatim. + @Test + public void stringDefaultWithSpecialCharactersRoundTrips() { + String custom = " a,b "; + FieldSpec spec = parse(FieldType.DIMENSION, DataType.STRING, + CommonsConfigurationUtils.replaceSpecialCharacterInPropertyValue(custom)); + assertEquals(spec.getDefaultNullValue(), custom); + assertEquals(spec, new DimensionFieldSpec("col", DataType.STRING, true, custom)); + FieldSpec typeDefault = parse(FieldType.DIMENSION, DataType.STRING, + CommonsConfigurationUtils.replaceSpecialCharacterInPropertyValue( + FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_STRING)); + assertSame(typeDefault.getDefaultNullValue(), FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_STRING); + } + + @Test + public void dateTimeDefaultsAndFormatStrings() { + PropertiesConfiguration config = configFor(FieldType.DATE_TIME, DataType.LONG, + DataType.LONG.toString(FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_LONG)); + DateTimeFieldSpec spec = (DateTimeFieldSpec) ColumnMetadataImpl.extractFieldSpec("col", config); + assertSame(spec.getDefaultNullValue(), FieldSpec.DEFAULT_DIMENSION_NULL_VALUE_OF_LONG); + assertEquals(spec, new DateTimeFieldSpec("col", DataType.LONG, DATETIME_FORMAT, DATETIME_GRANULARITY)); + // The format and granularity are stored as fresh strings and come back as the interned instances. + assertSame(spec.getFormat(), DATETIME_FORMAT); + assertSame(spec.getGranularity(), DATETIME_GRANULARITY); + + DateTimeFieldSpec custom = + (DateTimeFieldSpec) ColumnMetadataImpl.extractFieldSpec("col", configFor(FieldType.DATE_TIME, DataType.LONG, + "0")); + assertEquals(custom, + new DateTimeFieldSpec("col", DataType.LONG, DATETIME_FORMAT, DATETIME_GRANULARITY, 0L, null)); + assertEquals(custom.getDefaultNullValue(), 0L); + } + + /// `/tables/{table}/segments/{segment}/metadata` bean-serializes the FieldSpec, i.e. `getDefaultNullValue()` by + /// value, so its payload is byte-identical to the one a spec built straight from the literal (the + /// pre-canonicalization shape) produces, for every type including BYTES and UUID. Only `FieldSpec#toJsonObject()`, + /// which compares the value against the type default by identity, now omits a redundant BYTES default that used to + /// be emitted; no endpoint serializes a segment-derived schema that way. + @Test + public void segmentMetadataJsonUnchangedByCanonicalization() + throws Exception { + for (DataType dataType : new DataType[] { + DataType.INT, DataType.LONG, DataType.FLOAT, DataType.DOUBLE, DataType.BOOLEAN, DataType.TIMESTAMP, + DataType.STRING, DataType.JSON, DataType.BYTES, DataType.UUID, DataType.BIG_DECIMAL + }) { + Object constant = FieldSpec.getDefaultNullValue(FieldType.DIMENSION, dataType, null); + String literal = dataType.toString(constant); + FieldSpec parsed = parse(FieldType.DIMENSION, dataType, writtenLiteral(dataType, constant)); + FieldSpec legacy = new DimensionFieldSpec("col", dataType, true, literal); + assertEquals(JsonUtils.objectToString(parsed), JsonUtils.objectToString(legacy), dataType.name()); + assertEquals(parsed, legacy, dataType.name()); + } + assertFalse(parse(FieldType.DIMENSION, DataType.BYTES, "").toJsonObject().has("defaultNullValue")); + } + + /// A combination with no type default (rejected by schema validation, but constructible with an explicit default) + /// keeps parsing the literal instead of failing on the type-default lookup. + @Test + public void literalWithoutTypeDefaultIsKept() { + assertSame(ColumnMetadataImpl.canonicalDefaultNullValue(FieldType.METRIC, DataType.BOOLEAN, new String("1")), + "1"); + assertEquals(parse(FieldType.METRIC, DataType.BOOLEAN, "1").getDefaultNullValue(), 1); + } + + /// The strings a column retains for its lifetime alias the JVM-wide interned instances, so every segment of the + /// table shares them. + @Test + public void columnNameAndParentColumnAreInterned() { + PropertiesConfiguration config = baseConfig("metrics$cpu"); + config.setProperty(Column.getKeyFor("metrics$cpu", Column.COLUMN_NAME), new String("cpu")); + config.setProperty(Column.getKeyFor("metrics$cpu", Column.PARENT_COLUMN), new String("metrics")); + + ColumnMetadataImpl metadata = ColumnMetadataImpl.fromPropertiesConfiguration(config, 1, "metrics$cpu"); + + assertSame(metadata.getFieldSpec().getName(), "cpu"); + assertSame(metadata.getParentColumn(), "metrics"); + // Without an explicit COLUMN_NAME the key itself is the name. + String column = new String("plain"); + FieldSpec spec = ColumnMetadataImpl.extractFieldSpec(column, baseConfigWithoutName(column)); + assertSame(spec.getName(), "plain"); + } + + private static FieldSpec parse(FieldType fieldType, DataType dataType, @Nullable String defaultNullValue) { + return ColumnMetadataImpl.extractFieldSpec("col", configFor(fieldType, dataType, defaultNullValue)); + } + + /// The literal the segment creator writes for the given default null value. + private static String writtenLiteral(DataType dataType, Object defaultNullValue) { + String literal = dataType.toString(defaultNullValue); + return dataType.getStoredType() == DataType.STRING + ? CommonsConfigurationUtils.replaceSpecialCharacterInPropertyValue(literal) : literal; + } + + private static PropertiesConfiguration configFor(FieldType fieldType, DataType dataType, + @Nullable String defaultNullValue) { + PropertiesConfiguration config = baseConfig("col"); + config.setProperty(Column.getKeyFor("col", Column.COLUMN_TYPE), fieldType.name()); + config.setProperty(Column.getKeyFor("col", Column.DATA_TYPE), dataType.name()); + if (defaultNullValue != null) { + config.setProperty(Column.getKeyFor("col", Column.DEFAULT_NULL_VALUE), defaultNullValue); + } + if (fieldType == FieldType.DATE_TIME) { + config.setProperty(Column.getKeyFor("col", Column.DATETIME_FORMAT), new String(DATETIME_FORMAT)); + config.setProperty(Column.getKeyFor("col", Column.DATETIME_GRANULARITY), new String(DATETIME_GRANULARITY)); + } + return config; + } + private static PropertiesConfiguration baseConfig(String column) { - PropertiesConfiguration config = new PropertiesConfiguration(); + PropertiesConfiguration config = baseConfigWithoutName(column); config.setProperty(Column.getKeyFor(column, Column.COLUMN_NAME), column); + return config; + } + + private static PropertiesConfiguration baseConfigWithoutName(String column) { + PropertiesConfiguration config = new PropertiesConfiguration(); config.setProperty(Column.getKeyFor(column, Column.COLUMN_TYPE), FieldType.DIMENSION.name()); config.setProperty(Column.getKeyFor(column, Column.DATA_TYPE), DataType.STRING.name()); config.setProperty(Column.getKeyFor(column, Column.IS_SINGLE_VALUED), true);