Jackie-Jiang commented on code in PR #13212:
URL: https://github.com/apache/pinot/pull/13212#discussion_r1681829184
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java:
##########
@@ -812,6 +775,274 @@ private void createDerivedColumnV1Indices(String column,
FunctionEvaluator funct
}
}
+ /**
+ * Helper method to convert the output of a transform function to the
appropriate type for an SV or MV
+ * {@link FieldSpec.DataType#INT} field
+ *
+ * @param outputValue the output of the transform function
+ * @param isSingleValue true if the field (column) is single-valued
+ * @param outputValueType the output value type for the transform function;
can be null (in which case,
+ * the {@code outputValue} should be the field's
default null value)
+ * @param defaultNullValue the default null value for the field
+ * @param dictionary true if the column has a dictionary. For an MV field,
this results in a primitive array being
+ * returned rather than an array of the primitive wrapper
class
+ * @return the converted output value (either an Integer, an Integer[] or an
int[])
+ */
+ private Object getIntOutputValue(Object outputValue, boolean isSingleValue,
PinotDataType outputValueType,
+ Integer defaultNullValue, boolean dictionary) {
Review Comment:
Pass in primitive `int` for better performance. Same for other methods
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java:
##########
@@ -575,44 +587,59 @@ private void createDerivedColumnV1Indices(String column,
FunctionEvaluator funct
for (int j = 0; j < numArguments; j++) {
inputValues[j] = valueReaders.get(j).getValue(i);
}
- Object outputValue = functionEvaluator.evaluate(inputValues);
- outputValues[i] = outputValue;
- if (outputValueType == null) {
+
+ Object outputValue = null;
+ try {
+ outputValue = functionEvaluator.evaluate(inputValues);
+ } catch (Exception e) {
+ if (!errorOnFailure) {
+ LOGGER.debug("Encountered an exception while evaluating function
{} for derived column {} with "
+ + "arguments: {}", functionEvaluator, column,
Arrays.toString(inputValues), e);
+ functionEvaluateErrorCount++;
+ if (functionEvalError == null) {
+ functionEvalError = e;
+ inputValuesWithError = Arrays.copyOf(inputValues,
inputValues.length);
+ }
+ } else {
+ throw e;
+ }
+ }
+
+ if (outputValue == null) {
+ outputValue = fieldSpec.getDefaultNullValue();
Review Comment:
Should we consider not setting `outputValue`, but just leave it as `null`?
In the following handling, we can simply check `null` and fill with default
value
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
