alex-plekhanov commented on code in PR #10023:
URL: https://github.com/apache/ignite/pull/10023#discussion_r884899733
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/AccumulatorsFactory.java:
##########
@@ -300,15 +307,7 @@ private final class AccumulatorWrapperImpl implements
AccumulatorWrapper<Row> {
if (filterArg >= 0 && Boolean.TRUE != handler.get(filterArg, row))
return;
- Object[] args = new Object[argList.size()];
- for (int i = 0; i < argList.size(); i++) {
- args[i] = handler.get(argList.get(i), row);
-
- if (ignoreNulls && args[i] == null)
Review Comment:
Looks like we don't process ignoreNulls flag now. Shouldn't we process it by
Function created by createInAdapter method?
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/AccumulatorsFactory.java:
##########
@@ -226,17 +226,32 @@ private WrapperPrototype(AggregateCall call) {
List<Function<Object, Object>> casts =
Commons.transform(Pair.zip(inTypes, outTypes),
AccumulatorsFactory::cast);
- return new Function<Object[], Object[]>() {
- @Override public Object[] apply(Object[] args) {
- for (int i = 0; i < args.length; i++)
- args[i] = casts.get(i).apply(args[i]);
- return args;
+ return new Function<Row, Row>() {
+ final RowHandler<Row> hnd = ctx.rowHandler();
+
+ final RowHandler.RowFactory<Row> rowFac =
hnd.factory(ctx.getTypeFactory(), inputRowType);
+
+ @Override public Row apply(Row in) {
+ Row out = rowFac.create();
+
+ List<Integer> argList = call.getArgList();
+ for (int i = 0; i < hnd.columnCount(in); ++i) {
+ Object val = hnd.get(i, in);
+
+ int idx = argList.indexOf(i);
Review Comment:
Let's pre-calculate int array and use it instead of argList.indexOf
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java:
##########
@@ -972,39 +1027,174 @@ private ComparableMinMax(boolean min,
Function<IgniteTypeFactory, RelDataType> t
}
/** */
- private static class DistinctAccumulator implements Accumulator {
+ private static class SortingAccumulator<Row> implements Accumulator<Row> {
/** */
- private final Accumulator acc;
+ private final transient Comparator<Row> cmp;
/** */
- private final Set<Object> set = new HashSet<>();
+ private final List<Row> list;
/** */
- private DistinctAccumulator(Supplier<Accumulator> accSup) {
- this.acc = accSup.get();
+ private final Accumulator<Row> acc;
+
+ /**
+ * @param accSup Accumulator supplier.
+ * @param cmp Comparator.
+ */
+ private SortingAccumulator(Supplier<Accumulator<Row>> accSup,
Comparator<Row> cmp) {
+ this.cmp = cmp;
+
+ list = new ArrayList<>();
+ acc = accSup.get();
}
/** {@inheritDoc} */
- @Override public void add(Object... args) {
- Object in = args[0];
+ @Override public void add(Row row) {
+ list.add(row);
+ }
- if (in == null)
+ /** {@inheritDoc} */
+ @Override public void apply(Accumulator<Row> other) {
+ SortingAccumulator<Row> other1 = (SortingAccumulator<Row>)other;
+
+ list.addAll(other1.list);
+ }
+
+ /** {@inheritDoc} */
+ @Override public Object end() {
+ list.sort(cmp);
+
+ for (Row row : list)
+ acc.add(row);
+
+ return acc.end();
+ }
+
+ /** {@inheritDoc} */
+ @Override public List<RelDataType> argumentTypes(IgniteTypeFactory
typeFactory) {
+ return acc.argumentTypes(typeFactory);
+ }
+
+ /** {@inheritDoc} */
+ @Override public RelDataType returnType(IgniteTypeFactory typeFactory)
{
+ return acc.returnType(typeFactory);
+ }
+ }
+
+ /** */
+ private static class ListAggAccumulator<Row> extends
AbstractAccumulator<Row> {
+ /** Default separator. */
+ private static final String DEFAULT_SEPARATOR = ",";
+
+ /** */
+ private final List<Row> list;
+
+ /** */
+ private final boolean isDfltSep;
+
+ /** */
+ public ListAggAccumulator(AggregateCall aggCall, RowHandler<Row> hnd) {
+ super(aggCall, hnd);
+
+ isDfltSep = aggCall.getArgList().size() <= 1;
+
+ list = new ArrayList<>();
+ }
+
+ /** {@inheritDoc} */
+ @Override public void add(Row row) {
+ if (row == null || get(0, row) == null)
+ return;
+
+ list.add(row);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void apply(Accumulator<Row> other) {
+ ListAggAccumulator<Row> other0 = (ListAggAccumulator<Row>)other;
+
+ list.addAll(other0.list);
+ }
+
+ /** {@inheritDoc} */
+ @Override public Object end() {
+ if (list.isEmpty())
+ return null;
+
+ StringBuilder builder = new StringBuilder();
+
+ for (Row row: list) {
+ if (builder.length() != 0)
+ builder.append(extractSeparator(row));
+
+ builder.append(Objects.toString(get(0, row)));
+ }
+
+ return builder.toString();
+ }
+
+ /** */
+ private String extractSeparator(Row row) {
+ if (isDfltSep || columnCount(row) <= 1)
+ return DEFAULT_SEPARATOR;
+
+ Object rawSep = get(1, row);
+
+ if (rawSep == null)
+ return DEFAULT_SEPARATOR;
+
+ return rawSep.toString();
+ }
+
+ /** {@inheritDoc} */
+ @Override public List<RelDataType> argumentTypes(IgniteTypeFactory
typeFactory) {
+ return
F.asList(typeFactory.createTypeWithNullability(typeFactory.createSqlType(VARCHAR),
true),
+
typeFactory.createTypeWithNullability(typeFactory.createSqlType(CHAR), true));
+ }
+
+ /** {@inheritDoc} */
+ @Override public RelDataType returnType(IgniteTypeFactory typeFactory)
{
+ return
typeFactory.createTypeWithNullability(typeFactory.createSqlType(VARCHAR), true);
+ }
+ }
+
+ /** */
+ private static class DistinctAccumulator<Row> extends
AbstractAccumulator<Row> {
+ /** */
+ private final Accumulator<Row> acc;
+
+ /** */
+ private final Map<Object, Row> rows = new LinkedHashMap<>();
Review Comment:
Do we really need LinkedHashMap here? Why regular HashMap is not enough?
--
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]
