kgyrtkirk commented on code in PR #15127:
URL: https://github.com/apache/druid/pull/15127#discussion_r1358124448
##########
processing/src/test/java/org/apache/druid/segment/transform/TransformerTest.java:
##########
@@ -441,7 +445,74 @@ public void testTransformWithArrayDoubleInputs()
Assert.assertEquals(2.3, (Double) raw[1], 0.0);
Assert.assertNull(raw[2]);
Assert.assertEquals(3.4, (Double) raw[3], 0.0);
- Assert.assertEquals(Arrays.asList("1.2", "2.3", "null", "3.4"),
actual.getDimension("dim"));
+ Assert.assertArrayEquals(new String[]{"1.2", "2.3", null, "3.4"},
actual.getDimension("dim").toArray());
Assert.assertEquals(row.getTimestamp(), actual.getTimestamp());
}
+
+ @Test
+ public void testTransformWithExpr()
+ {
+ final Transformer transformer = new Transformer(
+ new TransformSpec(
+ null,
+ ImmutableList.of(new ExpressionTransform("dim", "array_slice(dim,
0, 5)", TestExprMacroTable.INSTANCE))
+ )
+ );
+ final List<String> dimList = ImmutableList.of("a", "b", "c", "d", "e",
"f", "g");
+ final MapBasedRow row = new MapBasedRow(
+ DateTimes.nowUtc(),
+ ImmutableMap.of("dim", dimList)
+ );
+ Assert.assertEquals(row.getDimension("dim"), dimList);
+ Assert.assertEquals(row.getRaw("dim"), dimList);
+
+ final InputRow actualTranformedRow = transformer.transform(new InputRow()
Review Comment:
I wonder if its necessary to use an inline `new InputRow()` here - do you
need to retain the `row` in that type for some reason ?
you could make public/move outside/etc [this
metod](https://github.com/apache/druid/blob/f0a70fe3c4533c889b6185774f104c0265a0c37e/indexing-hadoop/src/main/java/org/apache/druid/indexer/IndexGeneratorJob.java#L494)
...or probably try to use
[MapBasedInputRow](https://github.com/apache/druid/blob/f0a70fe3c4533c889b6185774f104c0265a0c37e/processing/src/main/java/org/apache/druid/data/input/MapBasedInputRow.java#L38)
?
##########
processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java:
##########
@@ -111,9 +110,7 @@ public Object eval(final Row row)
public List<String> evalDimension(Row row)
{
try {
- return Rows.objectToStrings(
Review Comment:
note: removal of `coerceEvalToObjectOrList` will have a sideeffect that it
will not anymore turn `[singleVal]` into just `singleVal` - I don't know if
that's important or not - but I wonder how `array_slice(dim, 0, 1)` is handled
before/after the patch; might worth a quick check
--
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]
