gianm commented on a change in pull request #11884:
URL: https://github.com/apache/druid/pull/11884#discussion_r745064415
##########
File path:
processing/src/main/java/org/apache/druid/segment/RowBasedColumnSelectorFactory.java
##########
@@ -340,9 +330,67 @@ public Class classOfObject()
@Override
public void inspectRuntimeShape(RuntimeShapeInspector inspector)
{
- inspector.visit("row", supplier);
+ inspector.visit("row", rowSupplier);
inspector.visit("extractionFn", extractionFn);
}
+
+ private void updateCurrentValues()
+ {
+ if (rowIdSupplier == null || rowIdSupplier.getAsLong() != currentId)
{
+ try {
+ final Object rawValue = dimFunction.apply(rowSupplier.get());
+
+ if (rawValue == null || rawValue instanceof String) {
+ final String s = NullHandling.emptyToNullIfNeeded((String)
rawValue);
+
+ if (extractionFn == null) {
+ dimensionValues = Collections.singletonList(s);
+ } else {
+ dimensionValues =
Collections.singletonList(extractionFn.apply(s));
+ }
+ } else if (rawValue instanceof List) {
+ // Consistent behavior with Rows.objectToStrings, but applies
extractionFn too.
+ //noinspection rawtypes
+ final List<String> values = new ArrayList<>(((List)
rawValue).size());
+
+ //noinspection rawtypes
+ for (final Object item : ((List) rawValue)) {
+ // Behavior with null item is to convert it to string
"null". This is not what most other areas of Druid
+ // would do when treating a null as a string, but it's
consistent with Rows.objectToStrings, which is
+ // commonly used when retrieving strings from input-row-like
objects.
+ if (extractionFn == null) {
+
values.add(NullHandling.emptyToNullIfNeeded(String.valueOf(item)));
Review comment:
My goal here was to retain the old behavior of
RowBasedColumnSelectorFactory (weird as it is), and just add the caching
optimization. So I'd like to leave the behavior toggles to a different patch,
if you don't mind. I haven't thought about it enough to have a good proposal
right now.
Although, about retaining the old behavior: after looking into it more, I
realized this patch isn't doing it. I need to move the
NullHandling.emptyToNullIfNeeded from here to lookupName to retain the current
behavior. So, I'll do that for this patch.
--
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]
