swaminathanmanish commented on code in PR #14601:
URL: https://github.com/apache/pinot/pull/14601#discussion_r1871192791
##########
pinot-spi/src/main/java/org/apache/pinot/spi/recordtransformer/enricher/RecordEnricher.java:
##########
@@ -16,25 +16,26 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.spi.recordenricher;
+package org.apache.pinot.spi.recordtransformer.enricher;
-import java.util.List;
import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.recordtransformer.RecordTransformer;
/**
- * Interface for enriching records.
- * If a column with the same name as the input column already exists in the
record, it will be overwritten.
+ * Record enricher is a special {@link RecordTransformer} which is applied
before other transformers to enrich the
+ * columns. If a column with the same name as the input column already exists
in the record, it will be overwritten.
*/
-public interface RecordEnricher {
- /**
- * Returns the list of input columns required for enriching the record.
- * This is used to make sure the required input fields are extracted.
- */
- List<String> getInputColumns();
+public interface RecordEnricher extends RecordTransformer {
/**
* Enriches the given record, by adding new columns to the same record.
*/
void enrich(GenericRow record);
Review Comment:
Do we even need this enrich interface and method ? Why not go all the way
and make these enrichers transformers
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/CompositeTransformer.java:
##########
@@ -82,12 +90,37 @@ public class CompositeTransformer implements
RecordTransformer {
* </ul>
*/
public static List<RecordTransformer> getDefaultTransformers(TableConfig
tableConfig, Schema schema) {
- return Stream.of(new ExpressionTransformer(tableConfig, schema), new
FilterTransformer(tableConfig),
- new SchemaConformingTransformer(tableConfig, schema),
- new SchemaConformingTransformerV2(tableConfig, schema), new
DataTypeTransformer(tableConfig, schema),
- new TimeValidationTransformer(tableConfig, schema), new
SpecialValueTransformer(schema),
- new NullValueTransformer(tableConfig, schema), new
SanitizationTransformer(schema)).filter(t -> !t.isNoOp())
- .collect(Collectors.toList());
+ List<RecordTransformer> transformers = new ArrayList<>();
+ IngestionConfig ingestionConfig = tableConfig.getIngestionConfig();
+ if (ingestionConfig != null) {
+ List<EnrichmentConfig> enrichmentConfigs =
ingestionConfig.getEnrichmentConfigs();
Review Comment:
We could have avoided creating this separate enricher config itself ?
Unfortunately we cannot remove it now.
--
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]
