Jackie-Jiang commented on code in PR #18977:
URL: https://github.com/apache/pinot/pull/18977#discussion_r4020596966
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java:
##########
@@ -283,6 +283,15 @@ public static ImmutableSegment load(SegmentDirectory
segmentDirectory, IndexLoad
/// segment format, adding new indices or updating default columns.
public static boolean needPreprocess(SegmentDirectory segmentDirectory,
IndexLoadingConfig indexLoadingConfig)
throws Exception {
+ return needPreprocess(segmentDirectory, indexLoadingConfig, true);
+ }
+
+ /// Same as [#needPreprocess(SegmentDirectory, IndexLoadingConfig)], with an
option to ignore transform-function
+ /// BACKFILL/UPDATE. Record-replay rebuild (RefreshSegment) must not treat
those as a rebuild signal; server
+ /// [SegmentPreProcessor] + DefaultColumnHandler is the apply path.
+ public static boolean needPreprocess(SegmentDirectory segmentDirectory,
IndexLoadingConfig indexLoadingConfig,
Review Comment:
The only place where `includeTransformFunctionActions` is passed as `false`
is from `RefreshSegemntTaskExecutor`, where the usage itself is wrong (there is
a TODO for it).
I do see the problem of backfilling always triggering segment refresh, bug
update should trigger refresh.
Based on this, I'd suggest removing backfill and only support updates on new
generated segment where transform function is persisted. This way the entire
flow can be simplified a lot.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java:
##########
@@ -138,6 +160,7 @@ boolean isRemoveAction() {
protected final TableConfig _tableConfig;
protected final Schema _schema;
protected final SegmentDirectory.Writer _segmentWriter;
+ private final Map<String, String> _transformFunctionByColumn;
Review Comment:
Keep it `protected` in base class
--
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]
