[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r378428398
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/any/NumericAnyBufferAggregator.java
##
@@ -32,11 +32,11 @@
public abstract class NumericAnyBufferAggregator
implements BufferAggregator
{
- private static final byte BYTE_FLAG_IS_NOT_SET = 0;
- private static final byte BYTE_FLAG_IS_SET = 1;
- private static final int IS_FOUND_FLAG_OFFSET_POSITION = 0;
- private static final int IS_NULL_FLAG_OFFSET_POSITION =
IS_FOUND_FLAG_OFFSET_POSITION + Byte.BYTES;
- private static final int FOUND_VALUE_OFFSET_POSITION =
IS_NULL_FLAG_OFFSET_POSITION + Byte.BYTES;
+ // Rightmost bit for is null check (0 for is null and 1 for not null)
+ // Second rightmost bit for is found check (0 for not found and 1 for found)
+ private static final byte BYTE_FLAG_FOUND_MASK = 0b0010;
Review comment:
Done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation URL: https://github.com/apache/druid/pull/9317#discussion_r378428020 ## File path: docs/querying/sql.md ## @@ -205,7 +205,7 @@ Only the COUNT aggregation can accept DISTINCT. |`EARLIEST(expr, maxBytesPerString)`|Like `EARLIEST(expr)`, but for strings. The `maxBytesPerString` parameter determines how much aggregation space to allocate per string. Strings longer than this limit will be truncated. This parameter should be set as low as possible, since high values will lead to wasted memory.| |`LATEST(expr)`|Returns the latest value of `expr`, which must be numeric. If `expr` comes from a relation with a timestamp column (like a Druid datasource) then "latest" is the value last encountered with the maximum overall timestamp of all values being aggregated. If `expr` does not come from a relation with a timestamp, then it is simply the last value encountered.| |`LATEST(expr, maxBytesPerString)`|Like `LATEST(expr)`, but for strings. The `maxBytesPerString` parameter determines how much aggregation space to allocate per string. Strings longer than this limit will be truncated. This parameter should be set as low as possible, since high values will lead to wasted memory.| -|`ANY_VALUE(expr)`|Returns any value of `expr`, which must be numeric. If `druid.generic.useDefaultValueForNull=true` this can return the default value for null and does not prefer "non-null" values over the default value for null. If `druid.generic.useDefaultValueForNull=false`, then this will return any value of `expr` including null| +|`ANY_VALUE(expr)`|Returns any value of `expr` including null. `expr` must be numeric. This aggregator simplify and optimize the performance by returning the first encoutnered value (including null)| Review comment: Added 'can'. The string any indicates "Like `ANY_VALUE(expr)`" which implies that whatever is mentioned for the `ANY_VALUE(expr)` also applies to the string any. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation URL: https://github.com/apache/druid/pull/9317#discussion_r378427106 ## File path: docs/querying/aggregations.md ## @@ -238,7 +238,7 @@ Note that queries with first/last aggregators on a segment created with rollup e (Double/Float/Long/String) ANY aggregator cannot be used in ingestion spec, and should only be specified as part of queries. -If `druid.generic.useDefaultValueForNull=true` aggregation can returns the default value for null and does not prefer "non-null" values over the default value for null. If `druid.generic.useDefaultValueForNull=false`, then aggregation will returns any value including null. +Returns any value including null. This aggregator simplify and optimize the performance by returning the first encoutnered value (including null) Review comment: oops done. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r378004999
##
File path: server/src/main/java/org/apache/druid/server/QueryLifecycle.java
##
@@ -318,14 +318,12 @@ public void emitLogsAndMetrics(
if (e != null) {
statsMap.put("exception", e.toString());
-
-if (e instanceof QueryInterruptedException) {
- // Mimic behavior from QueryResource, where this code was originally
taken from.
- log.noStackTrace().warn(e, "Exception while processing queryId
[%s]", baseQuery.getId());
- statsMap.put("interrupted", true);
- statsMap.put("reason", e.toString());
-}
+// Mimic behavior from QueryResource, where this code was originally
taken from.
Review comment:
Changed to just moving the logging out of the if 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r378005102
##
File path: server/src/main/java/org/apache/druid/server/QueryLifecycle.java
##
@@ -318,14 +318,12 @@ public void emitLogsAndMetrics(
if (e != null) {
statsMap.put("exception", e.toString());
-
-if (e instanceof QueryInterruptedException) {
- // Mimic behavior from QueryResource, where this code was originally
taken from.
- log.noStackTrace().warn(e, "Exception while processing queryId
[%s]", baseQuery.getId());
- statsMap.put("interrupted", true);
- statsMap.put("reason", e.toString());
-}
+// Mimic behavior from QueryResource, where this code was originally
taken from.
Review comment:
The change is so that we can see the exception (and possibly stacktrace) for
exceptions other than QueryInterruptedException too
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r378003007
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/any/NumericAnyBufferAggregator.java
##
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.aggregation.any;
+
+import org.apache.druid.common.config.NullHandling;
+import org.apache.druid.query.aggregation.BufferAggregator;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
+import org.apache.druid.segment.BaseNullableColumnValueSelector;
+
+import java.nio.ByteBuffer;
+
+/**
+ * Base type for buffer based 'any' aggregator for primitive numeric column
selectors
+ */
+public abstract class NumericAnyBufferAggregator
+implements BufferAggregator
+{
+ private static final byte BYTE_FLAG_IS_NOT_SET = 0;
+ private static final byte BYTE_FLAG_IS_SET = 1;
+ private static final int IS_FOUND_FLAG_OFFSET_POSITION = 0;
+ private static final int IS_NULL_FLAG_OFFSET_POSITION =
IS_FOUND_FLAG_OFFSET_POSITION + Byte.BYTES;
+ private static final int FOUND_VALUE_OFFSET_POSITION =
IS_NULL_FLAG_OFFSET_POSITION + Byte.BYTES;
Review comment:
Done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r378002862
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/any/NumericAnyBufferAggregator.java
##
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.aggregation.any;
+
+import org.apache.druid.common.config.NullHandling;
+import org.apache.druid.query.aggregation.BufferAggregator;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
+import org.apache.druid.segment.BaseNullableColumnValueSelector;
+
+import java.nio.ByteBuffer;
+
+/**
+ * Base type for buffer based 'any' aggregator for primitive numeric column
selectors
+ */
+public abstract class NumericAnyBufferAggregator
+implements BufferAggregator
+{
+ private static final byte BYTE_FLAG_IS_NOT_SET = 0;
Review comment:
Done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r378002826
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/any/NumericAnyBufferAggregator.java
##
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.aggregation.any;
+
+import org.apache.druid.common.config.NullHandling;
+import org.apache.druid.query.aggregation.BufferAggregator;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
+import org.apache.druid.segment.BaseNullableColumnValueSelector;
+
+import java.nio.ByteBuffer;
+
+/**
+ * Base type for buffer based 'any' aggregator for primitive numeric column
selectors
+ */
+public abstract class NumericAnyBufferAggregator
+implements BufferAggregator
+{
+ private static final byte BYTE_FLAG_IS_NOT_SET = 0;
+ private static final byte BYTE_FLAG_IS_SET = 1;
+ private static final int IS_FOUND_FLAG_OFFSET_POSITION = 0;
Review comment:
Done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r377997282
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/any/DoubleAnyAggregator.java
##
@@ -19,44 +19,35 @@
package org.apache.druid.query.aggregation.any;
-import org.apache.druid.query.aggregation.Aggregator;
-import org.apache.druid.query.aggregation.NullableNumericAggregator;
-import org.apache.druid.query.aggregation.NullableNumericAggregatorFactory;
import org.apache.druid.segment.BaseDoubleColumnValueSelector;
+import javax.annotation.Nullable;
+
/**
- * This Aggregator is created by the {@link DoubleAnyAggregatorFactory} which
extends from
- * {@link NullableNumericAggregatorFactory}. If null needs to be handle, then
{@link NullableNumericAggregatorFactory}
- * will wrap this aggregator in {@link NullableNumericAggregator} and can
handle all null in that class.
- * Hence, no null will ever be pass into this aggregator from the
valueSelector.
+ * This Aggregator is created by the {@link DoubleAnyAggregatorFactory} which
has no special null handling logic.
Review comment:
Removed
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r377996826
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/any/DoubleAnyAggregatorFactory.java
##
@@ -19,103 +19,214 @@
package org.apache.druid.query.aggregation.any;
-import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.druid.math.expr.ExprMacroTable;
+import com.google.common.base.Preconditions;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.query.aggregation.AggregateCombiner;
import org.apache.druid.query.aggregation.Aggregator;
import org.apache.druid.query.aggregation.AggregatorFactory;
import org.apache.druid.query.aggregation.AggregatorUtil;
import org.apache.druid.query.aggregation.BufferAggregator;
-import org.apache.druid.query.aggregation.SimpleDoubleAggregatorFactory;
+import org.apache.druid.query.aggregation.DoubleSumAggregator;
import org.apache.druid.query.cache.CacheKeyBuilder;
import org.apache.druid.segment.BaseDoubleColumnValueSelector;
+import org.apache.druid.segment.ColumnSelectorFactory;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.column.ColumnHolder;
import javax.annotation.Nullable;
+import java.nio.ByteBuffer;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
+import java.util.Objects;
-public class DoubleAnyAggregatorFactory extends SimpleDoubleAggregatorFactory
+public class DoubleAnyAggregatorFactory extends AggregatorFactory
{
+ private static final Comparator VALUE_COMPARATOR =
Comparator.nullsFirst(Double::compare);
+
+ private static final Aggregator NIL_AGGREGATOR = new DoubleAnyAggregator(
+ NilColumnValueSelector.instance()
+ )
+ {
+@Override
+public void aggregate()
+{
+ // no-op
+}
+ };
+
+ private static final BufferAggregator NIL_BUFFER_AGGREGATOR = new
DoubleAnyBufferAggregator(
+ NilColumnValueSelector.instance()
+ )
+ {
+@Override
+public void aggregate(ByteBuffer buf, int position)
+{
+ // no-op
+}
+ };
+
+ private final String fieldName;
+ private final String name;
+ private final boolean storeDoubleAsFloat;
+
@JsonCreator
public DoubleAnyAggregatorFactory(
@JsonProperty("name") String name,
- @JsonProperty("fieldName") final String fieldName,
- @JsonProperty("expression") @Nullable String expression,
- @JacksonInject ExprMacroTable macroTable
+ @JsonProperty("fieldName") final String fieldName
)
{
-super(macroTable, name, fieldName, expression);
- }
+Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator
name");
+Preconditions.checkNotNull(fieldName, "Must have a valid, non-null
fieldName");
- public DoubleAnyAggregatorFactory(String name, String fieldName)
- {
-this(name, fieldName, null, ExprMacroTable.nil());
+this.name = name;
+this.fieldName = fieldName;
+this.storeDoubleAsFloat = ColumnHolder.storeDoubleAsFloat();
}
@Override
- protected double nullValue()
+ public Aggregator factorize(ColumnSelectorFactory metricFactory)
{
-return Double.NaN;
+final BaseDoubleColumnValueSelector valueSelector =
metricFactory.makeColumnValueSelector(fieldName);
+if (valueSelector instanceof NilColumnValueSelector) {
+ return NIL_AGGREGATOR;
+} else {
+ return new DoubleAnyAggregator(
+ valueSelector
+ );
+}
}
@Override
- protected Aggregator buildAggregator(BaseDoubleColumnValueSelector selector)
+ public BufferAggregator factorizeBuffered(ColumnSelectorFactory
metricFactory)
{
-return new DoubleAnyAggregator(selector);
+final BaseDoubleColumnValueSelector valueSelector =
metricFactory.makeColumnValueSelector(fieldName);
+if (valueSelector instanceof NilColumnValueSelector) {
+ return NIL_BUFFER_AGGREGATOR;
+} else {
+ return new DoubleAnyBufferAggregator(
+ valueSelector
+ );
+}
}
@Override
- protected BufferAggregator
buildBufferAggregator(BaseDoubleColumnValueSelector selector)
+ public Comparator getComparator()
{
-return new DoubleAnyBufferAggregator(selector);
+return VALUE_COMPARATOR;
}
@Override
@Nullable
public Object combine(@Nullable Object lhs, @Nullable Object rhs)
{
-if (lhs != null) {
- return lhs;
-} else {
- return rhs;
-}
+return lhs;
+ }
+
+ @Override
+ public AggregateCombiner makeAggregateCombiner()
+ {
+throw new UOE("DoubleAnyAggregatorFactory is not supported during
ingestion for rollup");
}
@Override
public AggregatorFactory getCombiningFactory()
{
-return new DoubleAnyAggregatorFactory(name, name, null,
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r377977604
##
File path:
processing/src/main/java/org/apache/druid/query/aggregation/any/DoubleAnyAggregatorFactory.java
##
@@ -19,103 +19,214 @@
package org.apache.druid.query.aggregation.any;
-import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.druid.math.expr.ExprMacroTable;
+import com.google.common.base.Preconditions;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.query.aggregation.AggregateCombiner;
import org.apache.druid.query.aggregation.Aggregator;
import org.apache.druid.query.aggregation.AggregatorFactory;
import org.apache.druid.query.aggregation.AggregatorUtil;
import org.apache.druid.query.aggregation.BufferAggregator;
-import org.apache.druid.query.aggregation.SimpleDoubleAggregatorFactory;
+import org.apache.druid.query.aggregation.DoubleSumAggregator;
import org.apache.druid.query.cache.CacheKeyBuilder;
import org.apache.druid.segment.BaseDoubleColumnValueSelector;
+import org.apache.druid.segment.ColumnSelectorFactory;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.column.ColumnHolder;
import javax.annotation.Nullable;
+import java.nio.ByteBuffer;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
+import java.util.Objects;
-public class DoubleAnyAggregatorFactory extends SimpleDoubleAggregatorFactory
+public class DoubleAnyAggregatorFactory extends AggregatorFactory
{
+ private static final Comparator VALUE_COMPARATOR =
Comparator.nullsFirst(Double::compare);
+
+ private static final Aggregator NIL_AGGREGATOR = new DoubleAnyAggregator(
+ NilColumnValueSelector.instance()
+ )
+ {
+@Override
+public void aggregate()
+{
+ // no-op
+}
+ };
+
+ private static final BufferAggregator NIL_BUFFER_AGGREGATOR = new
DoubleAnyBufferAggregator(
+ NilColumnValueSelector.instance()
+ )
+ {
+@Override
+public void aggregate(ByteBuffer buf, int position)
+{
+ // no-op
+}
+ };
+
+ private final String fieldName;
+ private final String name;
+ private final boolean storeDoubleAsFloat;
+
@JsonCreator
public DoubleAnyAggregatorFactory(
@JsonProperty("name") String name,
- @JsonProperty("fieldName") final String fieldName,
- @JsonProperty("expression") @Nullable String expression,
- @JacksonInject ExprMacroTable macroTable
+ @JsonProperty("fieldName") final String fieldName
)
{
-super(macroTable, name, fieldName, expression);
- }
+Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator
name");
+Preconditions.checkNotNull(fieldName, "Must have a valid, non-null
fieldName");
- public DoubleAnyAggregatorFactory(String name, String fieldName)
- {
-this(name, fieldName, null, ExprMacroTable.nil());
+this.name = name;
+this.fieldName = fieldName;
+this.storeDoubleAsFloat = ColumnHolder.storeDoubleAsFloat();
}
@Override
- protected double nullValue()
+ public Aggregator factorize(ColumnSelectorFactory metricFactory)
{
-return Double.NaN;
+final BaseDoubleColumnValueSelector valueSelector =
metricFactory.makeColumnValueSelector(fieldName);
+if (valueSelector instanceof NilColumnValueSelector) {
+ return NIL_AGGREGATOR;
+} else {
+ return new DoubleAnyAggregator(
+ valueSelector
+ );
+}
}
@Override
- protected Aggregator buildAggregator(BaseDoubleColumnValueSelector selector)
+ public BufferAggregator factorizeBuffered(ColumnSelectorFactory
metricFactory)
{
-return new DoubleAnyAggregator(selector);
+final BaseDoubleColumnValueSelector valueSelector =
metricFactory.makeColumnValueSelector(fieldName);
+if (valueSelector instanceof NilColumnValueSelector) {
+ return NIL_BUFFER_AGGREGATOR;
+} else {
+ return new DoubleAnyBufferAggregator(
+ valueSelector
+ );
+}
}
@Override
- protected BufferAggregator
buildBufferAggregator(BaseDoubleColumnValueSelector selector)
+ public Comparator getComparator()
{
-return new DoubleAnyBufferAggregator(selector);
+return VALUE_COMPARATOR;
}
@Override
@Nullable
public Object combine(@Nullable Object lhs, @Nullable Object rhs)
{
-if (lhs != null) {
- return lhs;
-} else {
- return rhs;
-}
+return lhs;
Review comment:
to be consistent with the new policy of not discriminating null values
#equality
Decided not to make assumption in preferring non-null value.
This is an automated message from the Apache Git Service.
To respond to the message,
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation URL: https://github.com/apache/druid/pull/9317#discussion_r377971577 ## File path: docs/querying/sql.md ## @@ -205,7 +205,7 @@ Only the COUNT aggregation can accept DISTINCT. |`EARLIEST(expr, maxBytesPerString)`|Like `EARLIEST(expr)`, but for strings. The `maxBytesPerString` parameter determines how much aggregation space to allocate per string. Strings longer than this limit will be truncated. This parameter should be set as low as possible, since high values will lead to wasted memory.| |`LATEST(expr)`|Returns the latest value of `expr`, which must be numeric. If `expr` comes from a relation with a timestamp column (like a Druid datasource) then "latest" is the value last encountered with the maximum overall timestamp of all values being aggregated. If `expr` does not come from a relation with a timestamp, then it is simply the last value encountered.| |`LATEST(expr, maxBytesPerString)`|Like `LATEST(expr)`, but for strings. The `maxBytesPerString` parameter determines how much aggregation space to allocate per string. Strings longer than this limit will be truncated. This parameter should be set as low as possible, since high values will lead to wasted memory.| -|`ANY_VALUE(expr)`|Returns any value of `expr`, which must be numeric. If `druid.generic.useDefaultValueForNull=true` this can return the default value for null and does not prefer "non-null" values over the default value for null. If `druid.generic.useDefaultValueForNull=false`, then this will return any non-null value of `expr`| +|`ANY_VALUE(expr)`|Returns any value of `expr`, which must be numeric. If `druid.generic.useDefaultValueForNull=true` this can return the default value for null and does not prefer "non-null" values over the default value for null. If `druid.generic.useDefaultValueForNull=false`, then this will return any value of `expr` including null| Review comment: simplified as suggested. expr needs to be null for ANY_VALUE(expr) String column have to be ANY_VALUE(expr, maxBytesPerString) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation
maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation URL: https://github.com/apache/druid/pull/9317#discussion_r377965421 ## File path: docs/querying/aggregations.md ## @@ -238,7 +238,7 @@ Note that queries with first/last aggregators on a segment created with rollup e (Double/Float/Long/String) ANY aggregator cannot be used in ingestion spec, and should only be specified as part of queries. -If `druid.generic.useDefaultValueForNull=true` aggregation can returns the default value for null and does not prefer "non-null" values over the default value for null. If `druid.generic.useDefaultValueForNull=false`, then aggregation will returns any non-null value. +If `druid.generic.useDefaultValueForNull=true` aggregation can returns the default value for null and does not prefer "non-null" values over the default value for null. If `druid.generic.useDefaultValueForNull=false`, then aggregation will returns any value including null. Review comment: Added the why (short version of what Suneet wrote) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
