Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix incorrect comparator usage in FinalizingFieldAccessPostAggregator #16555

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Fix incorrect comparator usage in FinalizingFieldAccessPostAggregator #16555

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
31ffbba
add test
kgyrtkirk Jun 5, 2024
bd47fe7
fix finalizingfieldaccesspostaggregator
kgyrtkirk Jun 5, 2024
7e0eb34
use iq test instead of CQT
kgyrtkirk Jun 5, 2024
32b3978
add serde for histogram/histogramvisual
kgyrtkirk Jun 7, 2024
2ced982
undo add type arg
kgyrtkirk Jun 7, 2024
d662a62
more correct type in histogram module
kgyrtkirk Jun 7, 2024
e456fa4
cleanup
kgyrtkirk Jun 7, 2024
f0a1112
add serde for histo
kgyrtkirk Jun 7, 2024
99d2977
Merge remote-tracking branch 'apache/master' into fix-finalize-field-…
kgyrtkirk Jun 10, 2024
f6cede6
make basic serde for tminmax
kgyrtkirk Jun 10, 2024
c74481a
permit nulls; restore stuff for unknown inputs
kgyrtkirk Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
permit nulls; restore stuff for unknown inputs
  • Loading branch information
@kgyrtkirk
kgyrtkirk committed Jun 10, 2024
commit c74481ab5dd35b7820e581cbdf632afb2d4e3d22
10 changes: 7 additions & 3 deletions ...ain/java/org/apache/druid/query/aggregation/post/FinalizingFieldAccessPostAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.Sets;
import org.apache.druid.error.DruidException;
import org.apache.commons.collections.comparators.NullComparator;
import org.apache.druid.java.util.common.guava.Comparators;
import org.apache.druid.query.aggregation.AggregatorFactory;
import org.apache.druid.query.aggregation.PostAggregator;
import org.apache.druid.query.cache.CacheKeyBuilder;
Expand Down Expand Up @@ -123,9 +124,12 @@ public FinalizingFieldAccessPostAggregator decorate(final Map<String, Aggregator
if (aggregators != null && aggregators.containsKey(fieldName)) {
theFinalizer = aggregators.get(fieldName)::finalizeComputation;
finalizedType = aggregators.get(fieldName).getResultType();
theComparator = ColumnTypeFactory.getInstance().getTypeStrategy(finalizedType);
theComparator = new NullComparator(ColumnTypeFactory.getInstance().getTypeStrategy(finalizedType));
} else {
throw DruidException.defensive("PostAggregator input column [%s] not found in aggregators!", fieldName);
// noinspection unchecked
theComparator = (Comparator) Comparators.naturalNullsFirst();
theFinalizer = Function.identity();
finalizedType = null;
}

return new FinalizingFieldAccessPostAggregator(
Expand Down
1 change: 0 additions & 1 deletion ...java/org/apache/druid/query/aggregation/post/FinalizingFieldAccessPostAggregatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public void testComputedWithFinalizing()
{
String aggName = "biily";
AggregatorFactory aggFactory = EasyMock.createMock(AggregatorFactory.class);
EasyMock.expect(aggFactory.getComparator()).andReturn(Comparators.naturalNullsFirst()).once();
EasyMock.expect(aggFactory.finalizeComputation("test")).andReturn(3L).once();
EasyMock.expect(aggFactory.getResultType()).andReturn(ColumnType.LONG).once();
EasyMock.replay(aggFactory);
Expand Down
Loading