Jackie-Jiang commented on code in PR #12763:
URL: https://github.com/apache/pinot/pull/12763#discussion_r1548591606
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java:
##########
@@ -583,15 +583,31 @@ private void createDerivedColumnV1Indices(String column,
FunctionEvaluator funct
Object[] outputValues = new Object[numDocs];
PinotDataType outputValueType = null;
for (int i = 0; i < numDocs; i++) {
+ boolean hasNull = false;
for (int j = 0; j < numArguments; j++) {
inputValues[j] = valueReaders.get(j).getValue(i);
+ Object defaultNullValue =
argumentsMetadata.get(j).getFieldSpec().getDefaultNullValue();
+ if (inputValues[j] == null || inputValues[j] == defaultNullValue) {
Review Comment:
I don't think this check is necessary. When transform fails and
`errorOnFailure` is false, we can put `null` value
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java:
##########
@@ -583,15 +583,31 @@ private void createDerivedColumnV1Indices(String column,
FunctionEvaluator funct
Object[] outputValues = new Object[numDocs];
PinotDataType outputValueType = null;
for (int i = 0; i < numDocs; i++) {
+ boolean hasNull = false;
for (int j = 0; j < numArguments; j++) {
inputValues[j] = valueReaders.get(j).getValue(i);
+ Object defaultNullValue =
argumentsMetadata.get(j).getFieldSpec().getDefaultNullValue();
+ if (inputValues[j] == null || inputValues[j] == defaultNullValue) {
+ hasNull = true;
+ break;
+ }
}
- Object outputValue = functionEvaluator.evaluate(inputValues);
- outputValues[i] = outputValue;
- if (outputValueType == null) {
- Class<?> outputValueClass = outputValue.getClass();
- outputValueType = FunctionUtils.getArgumentType(outputValueClass);
- Preconditions.checkState(outputValueType != null, "Unsupported
output value class: %s", outputValueClass);
+ try {
+ Object outputValue = functionEvaluator.evaluate(inputValues);
+ outputValues[i] = outputValue;
+ if (outputValueType == null) {
+ Class<?> outputValueClass = outputValue.getClass();
+ outputValueType = FunctionUtils.getArgumentType(outputValueClass);
+ Preconditions.checkState(outputValueType != null, "Unsupported
output value class: %s", outputValueClass);
+ }
+ } catch (Exception e) {
+ if (!errorOnFailure && hasNull) {
+ LOGGER.warn("Caught exception while evaluating derived column: {}
with function: {} and arguments: {}",
Review Comment:
This can easily flood the log (one per row when an invalid transform is
provided)
--
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]
