github-code-scanning[bot] commented on code in PR #14462:
URL: https://github.com/apache/druid/pull/14462#discussion_r1237788492
##########
processing/src/main/java/org/apache/druid/query/aggregation/first/NumericFirstAggregator.java:
##########
@@ -49,21 +52,46 @@
/**
* Store the current primitive typed 'first' value
*/
- abstract void setCurrentValue();
+ abstract void setFirstValue(ColumnValueSelector valueSelector);
+
+ /**
+ * Store a non-null first value
+ */
+ abstract void setFirstValue(Number firstValue);
@Override
public void aggregate()
{
if (timeSelector.isNull()) {
return;
}
+
+ if (needsFoldCheck) {
+ final Object object = valueSelector.getObject();
+ if (object instanceof SerializablePair) {
+ SerializablePair<Long, Number> inPair = (SerializablePair<Long,
Number>) object;
+
+ if (inPair != null && inPair.lhs < firstTime) {
Review Comment:
## Useless null check
This check is useless. [inPair](1) cannot be null at this check, since it is
guarded by [...instanceof...](2).
[Show more
details](https://github.com/apache/druid/security/code-scanning/5146)
##########
processing/src/main/java/org/apache/druid/query/aggregation/last/NumericLastAggregator.java:
##########
@@ -19,47 +19,79 @@
package org.apache.druid.query.aggregation.last;
+import org.apache.druid.collections.SerializablePair;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.query.aggregation.Aggregator;
import org.apache.druid.segment.BaseLongColumnValueSelector;
-import org.apache.druid.segment.BaseNullableColumnValueSelector;
+import org.apache.druid.segment.ColumnValueSelector;
/**
* Base type for on heap 'last' aggregator for primitive numeric column
selectors..
*
* This could probably share a base class with {@link
org.apache.druid.query.aggregation.first.NumericFirstAggregator}
*/
-public abstract class NumericLastAggregator<TSelector extends
BaseNullableColumnValueSelector> implements Aggregator
+public abstract class NumericLastAggregator implements Aggregator
{
private final boolean useDefault = NullHandling.replaceWithDefault();
+
private final BaseLongColumnValueSelector timeSelector;
+ final ColumnValueSelector valueSelector;
+ final boolean needsFoldCheck;
- final TSelector valueSelector;
long lastTime;
boolean rhsNull;
- public NumericLastAggregator(BaseLongColumnValueSelector timeSelector,
TSelector valueSelector)
+ public NumericLastAggregator(BaseLongColumnValueSelector timeSelector,
ColumnValueSelector valueSelector, boolean needsFoldCheck)
{
this.timeSelector = timeSelector;
this.valueSelector = valueSelector;
+ this.needsFoldCheck = needsFoldCheck;
lastTime = Long.MIN_VALUE;
rhsNull = !useDefault;
}
+ /**
+ * Store the current primitive typed 'last' value
+ */
+ abstract void setLastValue(ColumnValueSelector valueSelector);
+
+ abstract void setLastValue(Number lastValue);
+
@Override
public void aggregate()
{
if (timeSelector.isNull()) {
return;
}
+
+ if (needsFoldCheck) {
+ final Object object = valueSelector.getObject();
+ if (object instanceof SerializablePair) {
+ final SerializablePair<Long, Number> inPair = (SerializablePair<Long,
Number>) object;
+
+ if (inPair != null && inPair.lhs >= lastTime) {
Review Comment:
## Useless null check
This check is useless. [inPair](1) cannot be null at this check, since it is
guarded by [...instanceof...](2).
[Show more
details](https://github.com/apache/druid/security/code-scanning/5148)
##########
processing/src/main/java/org/apache/druid/query/aggregation/first/NumericFirstBufferAggregator.java:
##########
@@ -89,11 +100,28 @@
if (timeSelector.isNull()) {
return;
}
- long time = timeSelector.getLong();
+
long firstTime = buf.getLong(position);
+ if (needsFoldCheck) {
+ final Object object = valueSelector.getObject();
+ if (object instanceof SerializablePair) {
+ final SerializablePair<Long, Number> inPair = (SerializablePair<Long,
Number>) object;
+ if (inPair != null && inPair.lhs < firstTime) {
Review Comment:
## Useless null check
This check is useless. [inPair](1) cannot be null at this check, since it is
guarded by [...instanceof...](2).
[Show more
details](https://github.com/apache/druid/security/code-scanning/5147)
##########
processing/src/main/java/org/apache/druid/query/aggregation/AbstractSerializableLongObjectPairSerde.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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;
+
+import org.apache.druid.collections.SerializablePair;
+import org.apache.druid.data.input.InputRow;
+import org.apache.druid.segment.GenericColumnSerializer;
+import org.apache.druid.segment.column.ColumnBuilder;
+import org.apache.druid.segment.data.GenericIndexed;
+import org.apache.druid.segment.serde.ComplexColumnPartSupplier;
+import org.apache.druid.segment.serde.ComplexMetricExtractor;
+import org.apache.druid.segment.serde.ComplexMetricSerde;
+import
org.apache.druid.segment.serde.LargeColumnSupportedComplexColumnSerializer;
+import org.apache.druid.segment.writeout.SegmentWriteOutMedium;
+
+import javax.annotation.Nullable;
+import java.nio.ByteBuffer;
+
+public abstract class AbstractSerializableLongObjectPairSerde<T extends
SerializablePair<Long, ?>> extends
+ ComplexMetricSerde
+{
+ private final Class<T> pairClass;
+
+ public AbstractSerializableLongObjectPairSerde(Class<T> pairClass)
+ {
+ this.pairClass = pairClass;
+ }
+
+ @Override
+ public ComplexMetricExtractor<?> getExtractor()
+ {
+ return new ComplexMetricExtractor<Object>()
+ {
+ @Override
+ public Class<T> extractedClass()
+ {
+ return pairClass;
+ }
+
+ @Nullable
+ @Override
+ public Object extractValue(InputRow inputRow, String metricName)
+ {
+ return inputRow.getMetric(metricName);
+ }
+ };
+ }
+
+ @Override
+ public void deserializeColumn(ByteBuffer buffer, ColumnBuilder columnBuilder)
+ {
+ final GenericIndexed<?> column = GenericIndexed.read(buffer,
getObjectStrategy(), columnBuilder.getFileMapper());
+ columnBuilder.setComplexColumnSupplier(new
ComplexColumnPartSupplier(getTypeName(), column));
+ }
+
+ @Override
+ public GenericColumnSerializer<T> getSerializer(SegmentWriteOutMedium
segmentWriteOutMedium, String column)
+ {
+ return LargeColumnSupportedComplexColumnSerializer.create(
+ segmentWriteOutMedium,
+ column,
+ getObjectStrategy()
Review Comment:
## Deprecated method or constructor invocation
Invoking [ComplexMetricSerde.getObjectStrategy](1) should be avoided because
it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5151)
##########
processing/src/main/java/org/apache/druid/query/aggregation/AbstractSerializableLongObjectPairSerde.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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;
+
+import org.apache.druid.collections.SerializablePair;
+import org.apache.druid.data.input.InputRow;
+import org.apache.druid.segment.GenericColumnSerializer;
+import org.apache.druid.segment.column.ColumnBuilder;
+import org.apache.druid.segment.data.GenericIndexed;
+import org.apache.druid.segment.serde.ComplexColumnPartSupplier;
+import org.apache.druid.segment.serde.ComplexMetricExtractor;
+import org.apache.druid.segment.serde.ComplexMetricSerde;
+import
org.apache.druid.segment.serde.LargeColumnSupportedComplexColumnSerializer;
+import org.apache.druid.segment.writeout.SegmentWriteOutMedium;
+
+import javax.annotation.Nullable;
+import java.nio.ByteBuffer;
+
+public abstract class AbstractSerializableLongObjectPairSerde<T extends
SerializablePair<Long, ?>> extends
+ ComplexMetricSerde
+{
+ private final Class<T> pairClass;
+
+ public AbstractSerializableLongObjectPairSerde(Class<T> pairClass)
+ {
+ this.pairClass = pairClass;
+ }
+
+ @Override
+ public ComplexMetricExtractor<?> getExtractor()
+ {
+ return new ComplexMetricExtractor<Object>()
+ {
+ @Override
+ public Class<T> extractedClass()
+ {
+ return pairClass;
+ }
+
+ @Nullable
+ @Override
+ public Object extractValue(InputRow inputRow, String metricName)
+ {
+ return inputRow.getMetric(metricName);
+ }
+ };
+ }
+
+ @Override
+ public void deserializeColumn(ByteBuffer buffer, ColumnBuilder columnBuilder)
+ {
+ final GenericIndexed<?> column = GenericIndexed.read(buffer,
getObjectStrategy(), columnBuilder.getFileMapper());
Review Comment:
## Deprecated method or constructor invocation
Invoking [ComplexMetricSerde.getObjectStrategy](1) should be avoided because
it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5150)
##########
processing/src/main/java/org/apache/druid/query/aggregation/last/NumericLastBufferAggregator.java:
##########
@@ -92,11 +103,29 @@
if (timeSelector.isNull()) {
return;
}
- long time = timeSelector.getLong();
+
long lastTime = buf.getLong(position);
+ if (needsFoldCheck) {
+ final Object object = valueSelector.getObject();
+ if (object instanceof SerializablePair) {
+ final SerializablePair<Long, Number> inPair = (SerializablePair<Long,
Number>) object;
+
+ if (inPair != null && inPair.lhs >= lastTime) {
Review Comment:
## Useless null check
This check is useless. [inPair](1) cannot be null at this check, since it is
guarded by [...instanceof...](2).
[Show more
details](https://github.com/apache/druid/security/code-scanning/5149)
--
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]
