Vamsi-klu commented on code in PR #18977:
URL: https://github.com/apache/pinot/pull/18977#discussion_r4051880393
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/BaseSegmentCreator.java:
##########
@@ -694,6 +707,35 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
}
}
+ /// Records the transform function used to generate the given column in the
segment metadata properties.
+ public static void addTransformFunction(PropertiesConfiguration properties,
String column,
+ @Nullable String transformFunction) {
+ if (transformFunction != null) {
+ String validTransformFunction =
+
CommonsConfigurationUtils.replaceSpecialCharacterInPropertyValue(transformFunction);
+ if (validTransformFunction != null) {
+ properties.setProperty(getKeyFor(column, TRANSFORM_FUNCTION),
validTransformFunction);
+ }
+ }
+ }
+
+ @Nullable
+ @SuppressWarnings("deprecation")
+ private String getTransformFunctionForColumn(String column) {
Review Comment:
Done. Segment creation now builds the column-to-transform map once through
`IngestionConfigUtils.getTransformFunctionByColumn`. The ingestion config takes
precedence, with schema transforms as the fallback. The default-column path
uses the same helper.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/ColumnMetadata.java:
##########
@@ -57,6 +57,12 @@ default boolean isNonNull() {
/// Returns `true` when the column is auto-generated by the default column
handler.
boolean isAutoGenerated();
+ /// Returns the transform function expression used to generate the column,
if persisted in the segment metadata.
+ @Nullable
+ default String getTransformFunction() {
Review Comment:
I kept compatibility defaults here because implementations that do not know
about provenance need to retain the legacy behavior. The defaults return
unknown provenance. `ColumnMetadataImpl`, `EmptyColumnMetadata`, and
`SimpleColumnMetadata` provide the concrete values. This prevents older
internal implementations from being treated as known no-transform columns.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -847,7 +875,8 @@ public ColumnMetadataImpl build() {
return new ColumnMetadataImpl(_fieldSpec, _totalDocs, _cardinality,
_hasDictionary, _forwardIndexEncoding,
_sorted, _nonNull, _minValue, _maxValue, _minMaxValueInvalid,
_lengthOfShortestElement,
_lengthOfLongestElement, _isAscii, _totalNumberOfEntries,
_maxNumberOfMultiValues, _maxRowLengthInBytes,
- _bitsPerElement, _partitionFunction, _partitions, _autoGenerated,
_parentColumn, _sparseKeys,
+ _bitsPerElement, _partitionFunction, _partitions, _autoGenerated,
_transformFunction, _parentColumn,
+ _sparseKeys,
Review Comment:
Reformatted this. Spotless and Checkstyle are clean on the affected modules.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -481,6 +494,15 @@ private static ChunkCompressionType
parseCompressionType(String column, @Nullabl
}
}
+ @Nullable
+ private static String extractTransformFunction(String column,
PropertiesConfiguration config) {
+ Object transformFunctionProperty =
config.getProperty(Column.getKeyFor(column, Column.TRANSFORM_FUNCTION));
Review Comment:
I kept `getProperty()` for the legacy raw key because `getString()` performs
Commons Configuration interpolation and can rewrite `${...}` inside a Groovy
expression. New metadata is stored as Base64-encoded UTF-8 and decoded
directly. Tests cover Unicode, `${...}`, commas, quotes, and backslashes.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java:
##########
@@ -111,20 +112,28 @@ protected enum DefaultColumnAction {
UPDATE_DIMENSION_DATA_TYPE,
UPDATE_DIMENSION_DEFAULT_VALUE,
UPDATE_DIMENSION_NUMBER_OF_VALUES,
+ UPDATE_DIMENSION_TRANSFORM_FUNCTION,
UPDATE_METRIC_DATA_TYPE,
UPDATE_METRIC_DEFAULT_VALUE,
UPDATE_METRIC_NUMBER_OF_VALUES,
+ UPDATE_METRIC_TRANSFORM_FUNCTION,
UPDATE_DATE_TIME_DATA_TYPE,
UPDATE_DATE_TIME_DEFAULT_VALUE,
+ UPDATE_DATE_TIME_TRANSFORM_FUNCTION,
UPDATE_COMPLEX_DATA_TYPE,
- UPDATE_COMPLEX_DEFAULT_VALUE;
+ UPDATE_COMPLEX_DEFAULT_VALUE,
+ UPDATE_COMPLEX_TRANSFORM_FUNCTION,
+ // Metadata-only action: record the configured transform function for an
auto-generated column created before the
+ // transform function was tracked in the segment metadata. No values are
regenerated, and it is handled entirely
+ // within updateDefaultColumns(), i.e. it is never dispatched to
updateDefaultColumn().
+ BACKFILL_TRANSFORM_FUNCTION;
Review Comment:
I reworked this into versioned provenance instead of adding another backfill
field. Legacy columns have no provenance version and remain untouched. Known
transforms and known no-transform states carry a version, so removing and later
restoring a transform rebuilds the values both times. Tests cover the legacy
no-op and the complete remove-and-restore sequence.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/BaseSegmentCreator.java:
##########
@@ -694,6 +700,52 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
}
}
+ /// Records a transform that actually produced the stored column values.
+ public static void addTransformFunction(PropertiesConfiguration properties,
String column,
+ @Nullable String transformFunction) {
+ addTransformFunction(properties, column, transformFunction, false);
+ }
+
+ /// Records transform provenance for a column.
+ /// A real stored transform is written to
[V1Constants.MetadataKeys.Column#TRANSFORM_FUNCTION]. A backward-compat
+ /// backfill (values were not regenerated from the expression) is written
only to
+ /// [V1Constants.MetadataKeys.Column#TRANSFORM_FUNCTION_BACKFILLED], so the
two cases stay distinguishable.
+ public static void addTransformFunction(PropertiesConfiguration properties,
String column,
+ @Nullable String transformFunction, boolean backfilled) {
+ String transformFunctionKey = getKeyFor(column, TRANSFORM_FUNCTION);
+ String backfilledKey = getKeyFor(column, TRANSFORM_FUNCTION_BACKFILLED);
+ if (transformFunction == null) {
+ properties.clearProperty(transformFunctionKey);
+ properties.clearProperty(backfilledKey);
+ return;
+ }
+ String validTransformFunction =
CommonsConfigurationUtils.replaceSpecialCharacterInPropertyValue(transformFunction);
+ if (validTransformFunction == null) {
+ throw new IllegalArgumentException("Cannot persist transform function
for column: " + column
+ + " because it contains UTF-16 surrogate characters that segment
metadata cannot store");
Review Comment:
Fixed by storing new transform metadata as Base64-encoded UTF-8. The raw
property remains only as a compatibility reader for earlier branch artifacts.
Tests cover emoji, `${...}`, commas, quotes, and backslashes.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java:
##########
@@ -159,6 +161,8 @@ public void process(@Nullable SegmentOperationsThrottlerSet
segmentOperationsThr
DefaultColumnHandler defaultColumnHandler =
DefaultColumnHandlerFactory.getDefaultColumnHandler(indexDir,
segmentMetadata, _indexLoadingConfig,
segmentWriter);
+ // Capture before apply: UPDATE_*_TRANSFORM_FUNCTION changes values;
BACKFILL does not.
+ _columnsWithChangedTransformValues =
defaultColumnHandler.getColumnsWithPendingTransformValueChanges();
Review Comment:
Fixed. A transform value change now rebuilds the shared multi-column text
index when the changed column participates in it. The regression test verifies
that the old term disappears and the new term becomes searchable.
--
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]
