Skip to content

Add optional extraMetadata map to FieldSpec #18984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
navina wants to merge 1 commit into
base: master
Choose a base branch
from
+123 −3
Draft

Add optional extraMetadata map to FieldSpec #18984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,14 @@ public enum MaxLengthExceedStrategy {
@Nullable
protected List<String> _aliases;

// Optional, free-form per-column metadata. It is additive, excluded from backward-compatibility
// checks, and omitted from serialization when unset/empty. The keys and their interpretation are
// defined by whoever populates it; the core schema attaches no semantics to it.
@JsonProperty("extraMetadata")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
@Nullable
protected Map<String, String> _extraMetadata;

protected String _name;
protected DataType _dataType;
protected boolean _singleValueField = true;
Expand Down Expand Up @@ -270,6 +278,15 @@ public void setAliases(@Nullable List<String> aliases) {
_aliases = aliases == null || aliases.isEmpty() ? null : aliases;
}

@Nullable
public Map<String, String> getExtraMetadata() {
return _extraMetadata;
}

public void setExtraMetadata(@Nullable Map<String, String> extraMetadata) {
_extraMetadata = extraMetadata == null || extraMetadata.isEmpty() ? null : extraMetadata;
}

public DataType getDataType() {
return _dataType;
}
Expand Down Expand Up @@ -624,7 +641,7 @@ public ObjectNode toJsonObject() {
return jsonObject;
}

/// Appends `fieldId` and `aliases` (when set) to the given JSON object.
/// Appends `fieldId`, `aliases`, and `extraMetadata` (when set) to the given JSON object.
///
/// Subclasses that build JSON without calling [FieldSpec#toJsonObject()], such as [TimeFieldSpec], use this helper
/// to preserve these fields during schema round-trip serialization.
Expand All @@ -639,6 +656,12 @@ protected void appendFieldIdAndAliases(ObjectNode jsonObject) {
}
jsonObject.set("aliases", aliasesArray);
}
if (_extraMetadata != null && !_extraMetadata.isEmpty()) {
ObjectNode metadataNode = jsonObject.putObject("extraMetadata");
for (Map.Entry<String, String> entry : _extraMetadata.entrySet()) {
metadataNode.put(entry.getKey(), entry.getValue());
}
}
}

protected void appendDefaultNullValue(ObjectNode jsonNode) {
Expand Down Expand Up @@ -716,14 +739,15 @@ public boolean equals(Object o) {
&& Objects.equals(_description, that._description)
&& Objects.equals(_tags, that._tags)
&& Objects.equals(_fieldId, that._fieldId)
&& Objects.equals(_aliases, that._aliases);
&& Objects.equals(_aliases, that._aliases)
&& Objects.equals(_extraMetadata, that._extraMetadata);
}

@Override
public int hashCode() {
return Objects.hash(_name, _dataType, _singleValueField, _notNull, _maxLength, _maxLengthExceedStrategy,
_allowTrailingZeros, _dataType.hashCode(_defaultNullValue), _transformFunction, _virtualColumnProvider,
_description, _tags, _fieldId, _aliases);
_description, _tags, _fieldId, _aliases, _extraMetadata);
}

/**
Expand Down
96 changes: 96 additions & 0 deletions pinot-spi/src/test/java/org/apache/pinot/spi/data/FieldSpecTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.TimeUnit;
import org.apache.pinot.spi.utils.ByteArray;
Expand Down Expand Up @@ -776,4 +777,99 @@ public void testFieldIdAndAliasesNotInBackwardCompatibility() {

assertThat(newSpec.isBackwardCompatibleWith(oldSpec)).isTrue();
}

@Test
public void testExtraMetadataSerdeRoundtrip()
throws Exception {
DimensionFieldSpec fieldSpec = new DimensionFieldSpec("addr", JSON, true);
fieldSpec.setExtraMetadata(Map.of("2", "person"));

String json = fieldSpec.toJsonObject().toString();
assertThat(json).contains("\"extraMetadata\":{\"2\":\"person\"}");

DimensionFieldSpec deserialized = JsonUtils.stringToObject(json, DimensionFieldSpec.class);
assertThat(deserialized.getExtraMetadata()).isEqualTo(Map.of("2", "person"));

String jacksonJson = JsonUtils.objectToString(fieldSpec);
DimensionFieldSpec fromJackson = JsonUtils.stringToObject(jacksonJson, DimensionFieldSpec.class);
assertThat(fromJackson.getExtraMetadata()).isEqualTo(Map.of("2", "person"));
}

@Test
public void testExtraMetadataOmittedWhenNotSet()
throws Exception {
DimensionFieldSpec fieldSpec = new DimensionFieldSpec("col1", INT, true);

String json = fieldSpec.toJsonObject().toString();
assertThat(json).as("extraMetadata should be absent when null").doesNotContain("extraMetadata");

String jacksonJson = JsonUtils.objectToString(fieldSpec);
assertThat(jacksonJson).as("extraMetadata should be absent when null").doesNotContain("extraMetadata");
}

@Test
public void testEmptyExtraMetadataOmittedFromJson()
throws Exception {
DimensionFieldSpec fieldSpec = new DimensionFieldSpec("col1", INT, true);
fieldSpec.setExtraMetadata(Map.of());
assertThat(fieldSpec.getExtraMetadata()).as("empty metadata normalizes to null").isNull();

String json = fieldSpec.toJsonObject().toString();
assertThat(json).as("empty extraMetadata should be absent").doesNotContain("extraMetadata");
assertThat(JsonUtils.stringToObject(json, DimensionFieldSpec.class)).isEqualTo(fieldSpec);

String jacksonJson = JsonUtils.objectToString(fieldSpec);
assertThat(jacksonJson).as("empty extraMetadata absent (Jackson)").doesNotContain("extraMetadata");
assertThat(JsonUtils.stringToObject(jacksonJson, DimensionFieldSpec.class)).isEqualTo(fieldSpec);
}

@Test
public void testTimeFieldSpecRoundTripsExtraMetadataThroughSchema()
throws Exception {
// TimeFieldSpec.toJsonObject() builds its JSON via the shared appendFieldIdAliasesAndMetadata helper;
// ensure extraMetadata round-trips through schema (de)serialization for legacy TIME columns too.
TimeFieldSpec timeFieldSpec = new TimeFieldSpec(new TimeGranularitySpec(LONG, TimeUnit.DAYS, "ts"));
timeFieldSpec.setExtraMetadata(Map.of("7", "event_ts"));

Schema schema = new Schema();
schema.setSchemaName("ts_schema");
schema.addField(timeFieldSpec);

String json = schema.toSingleLineJsonString();
assertThat(json).contains("\"extraMetadata\":{\"7\":\"event_ts\"}");

FieldSpec deserialized = Schema.fromString(json).getFieldSpecFor("ts");
assertThat(deserialized.getExtraMetadata()).isEqualTo(Map.of("7", "event_ts"));
}

@Test
public void testOldJsonWithoutExtraMetadataDeserializesCleanly()
throws Exception {
String oldJson = "{\"name\":\"col1\",\"dataType\":\"STRING\"}";
DimensionFieldSpec fieldSpec = JsonUtils.stringToObject(oldJson, DimensionFieldSpec.class);
assertThat(fieldSpec.getExtraMetadata()).isNull();
}

@Test
public void testExtraMetadataInEqualsAndHashCode() {
DimensionFieldSpec spec1 = new DimensionFieldSpec("col1", JSON, true);
DimensionFieldSpec spec2 = new DimensionFieldSpec("col1", JSON, true);
spec2.setExtraMetadata(Map.of("2", "person"));

assertThat(spec1).isNotEqualTo(spec2);
assertThat(spec1.hashCode()).isNotEqualTo(spec2.hashCode());

spec1.setExtraMetadata(Map.of("2", "person"));
assertThat(spec1).isEqualTo(spec2);
assertThat(spec1.hashCode()).isEqualTo(spec2.hashCode());
}

@Test
public void testExtraMetadataNotInBackwardCompatibility() {
DimensionFieldSpec oldSpec = new DimensionFieldSpec("col1", JSON, true);
DimensionFieldSpec newSpec = new DimensionFieldSpec("col1", JSON, true);
newSpec.setExtraMetadata(Map.of("2", "person"));

assertThat(newSpec.isBackwardCompatibleWith(oldSpec)).isTrue();
}
}
Loading