Re: [PR] Add pair Latest/Earliest aggregators (druid)
github-actions[bot] commented on PR #14195: URL: https://github.com/apache/druid/pull/14195#issuecomment-2121471234 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the d...@druid.apache.org list. Thank you for your contributions. -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[PR] Add pair Latest/Earliest aggregators (druid)
imply-cheddar opened a new pull request, #14195: URL: https://github.com/apache/druid/pull/14195 The current Latest/Earliest aggregators all do finalization to pretend like they are dealing with their payload type instead of exposing that they are actually dealing with pairs. This adds a STRING_LATEST and STRING_EARLIEST that explicitly deal with pairs and also adds PAIR_LEFT and PAIR_RIGHT functions that can access the two sides of the pairs. This does not add the numerical versions yet, though that can be an easy iteration on top of this. -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
github-actions[bot] closed pull request #14195: Add pair Latest/Earliest aggregators URL: https://github.com/apache/druid/pull/14195 -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
github-actions[bot] commented on PR #14195: URL: https://github.com/apache/druid/pull/14195#issuecomment-1989680188 This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
github-actions[bot] commented on PR #14195: URL: https://github.com/apache/druid/pull/14195#issuecomment-1937927751 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the d...@druid.apache.org list. Thank you for your contributions. -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295010735
##
processing/src/main/java/org/apache/druid/query/aggregation/last/StringLastAggregatorFactory.java:
##
@@ -87,37 +103,71 @@ public void aggregate(ByteBuffer buf, int position)
}
};
+ private final String name;
private final String fieldName;
private final String timeColumn;
- private final String name;
+ private final String foldColumn;
protected final int maxStringBytes;
+ private final boolean finalize;
@JsonCreator
public StringLastAggregatorFactory(
@JsonProperty("name") String name,
- @JsonProperty("fieldName") final String fieldName,
+ @JsonProperty("fieldName") @Nullable final String fieldName,
@JsonProperty("timeColumn") @Nullable final String timeColumn,
- @JsonProperty("maxStringBytes") Integer maxStringBytes
+ @JsonProperty("foldColumn") @Nullable final String foldColumn,
Review Comment:
same comment about `foldColumn` vs just using `ColumnCapabilities`
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295019287
##
processing/src/main/java/org/apache/druid/query/aggregation/pair/PairRightExprMacro.java:
##
@@ -0,0 +1,87 @@
+/*
+ * 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.pair;
+
+import org.apache.druid.collections.SerializablePair;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.math.expr.Expr;
+import org.apache.druid.math.expr.ExprEval;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.math.expr.ExprType;
+import org.apache.druid.math.expr.ExpressionType;
+import org.apache.druid.query.aggregation.first.StringFirstAggregatorFactory;
+
+import javax.annotation.Nullable;
+import java.util.List;
+
+public class PairRightExprMacro implements ExprMacroTable.ExprMacro
+{
+ @Override
+ public Expr apply(List args)
+ {
+validationHelperCheckArgumentCount(args, 1);
+Expr arg = args.get(0);
+
+class PairRightExpr extends
ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr
+{
+ public PairRightExpr(Expr arg)
+ {
+super("pair_right", arg);
+ }
+
+ @Override
+ public ExprEval eval(ObjectBinding bindings)
+ {
+Object pairResult = args.get(0).eval(bindings).value();
+if (pairResult instanceof SerializablePair){
+ return ExprEval.bestEffortOf(((SerializablePair) pairResult).rhs);
+}
+
+return ExprEval.of(null);
+ }
+
+ @Override
+ public Expr visit(Shuttle shuttle)
+ {
+return shuttle.visit(apply(shuttle.visitAll(args)));
+ }
+
+ @Nullable
+ @Override
+ public ExpressionType getOutputType(InputBindingInspector inspector)
+ {
+final ExpressionType inputType =
inspector.getType(arg.getBindingIfIdentifier());
+if (inputType.is(ExprType.COMPLEX)) {
+ if
(StringFirstAggregatorFactory.TYPE.getComplexTypeName().equals(inputType.getComplexTypeName()))
{
+return ExpressionType.STRING;
+ }
+}
+throw new UOE("Invalid input type [%s] to pair_right, expected a
complex pair type");
Review Comment:
same comment about using `validationFailed`
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195: URL: https://github.com/apache/druid/pull/14195#discussion_r1295027866 ## sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java: ## @@ -89,7 +91,7 @@ AggregatorFactory createAggregatorFactory(String name, String fieldName, String return new DoubleFirstAggregatorFactory(name, fieldName, timeColumn); case STRING: case COMPLEX: -return new StringFirstAggregatorFactory(name, fieldName, timeColumn, maxStringBytes); +return new StringFirstAggregatorFactory(name, fieldName, timeColumn, null, maxStringBytes, true); Review Comment: is this right if we go forward with separate `foldColumn`? Shouldn't the complex case put `fieldName` in the `foldColumn`? If you drop `foldColumn` and use the selector factory + capabilities (my preference at least at the time of writing this comment) instead then this doesn't need to change for now I think. Though, this part will also need to change if the PR that adds ingest time support for numeric first/last, since we'll need to get the complex type name and pick the correct folding aggregator... -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295018422
##
processing/src/main/java/org/apache/druid/query/aggregation/pair/PairLeftExprMacro.java:
##
@@ -0,0 +1,87 @@
+/*
+ * 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.pair;
+
+import org.apache.druid.collections.SerializablePair;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.math.expr.Expr;
+import org.apache.druid.math.expr.ExprEval;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.math.expr.ExprType;
+import org.apache.druid.math.expr.ExpressionType;
+import org.apache.druid.query.aggregation.first.StringFirstAggregatorFactory;
+
+import javax.annotation.Nullable;
+import java.util.List;
+
+public class PairLeftExprMacro implements ExprMacroTable.ExprMacro
+{
+ @Override
+ public Expr apply(List args)
+ {
+validationHelperCheckArgumentCount(args, 1);
+Expr arg = args.get(0);
+
+class PairLeft extends ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr
+{
+ public PairLeft(Expr arg)
+ {
+super("pair_left", arg);
+ }
+
+ @Override
+ public ExprEval eval(ObjectBinding bindings)
+ {
+Object pairResult = args.get(0).eval(bindings).value();
+if (pairResult instanceof SerializablePair){
+ return ExprEval.bestEffortOf(((SerializablePair) pairResult).lhs);
+}
+
+return ExprEval.of(null);
+ }
+
+ @Override
+ public Expr visit(Shuttle shuttle)
+ {
+return shuttle.visit(apply(shuttle.visitAll(args)));
+ }
+
+ @Nullable
+ @Override
+ public ExpressionType getOutputType(InputBindingInspector inspector)
+ {
+final ExpressionType inputType =
inspector.getType(arg.getBindingIfIdentifier());
+if (inputType.is(ExprType.COMPLEX)) {
+ if
(StringFirstAggregatorFactory.TYPE.getComplexTypeName().equals(inputType.getComplexTypeName()))
{
+return ExpressionType.LONG;
+ }
+}
+throw new UOE("Invalid input type [%s] to pair_left, expected a
complex pair type");
Review Comment:
macro expression implement an interface called `NamedFunction` that provide
a bunch of validation helper methods to help provide consistent error messaging
for expressions, so could do something like
```
validationFailed("Invalid input type [%s], expected [%s]", inputType,
EXPRESSION_TYPE)
```
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295015812
##
processing/src/main/java/org/apache/druid/query/aggregation/pair/PairLeftExprMacro.java:
##
@@ -0,0 +1,87 @@
+/*
+ * 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.pair;
+
+import org.apache.druid.collections.SerializablePair;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.math.expr.Expr;
+import org.apache.druid.math.expr.ExprEval;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.math.expr.ExprType;
+import org.apache.druid.math.expr.ExpressionType;
+import org.apache.druid.query.aggregation.first.StringFirstAggregatorFactory;
+
+import javax.annotation.Nullable;
+import java.util.List;
+
+public class PairLeftExprMacro implements ExprMacroTable.ExprMacro
+{
+ @Override
+ public Expr apply(List args)
+ {
+validationHelperCheckArgumentCount(args, 1);
+Expr arg = args.get(0);
+
+class PairLeft extends ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr
+{
+ public PairLeft(Expr arg)
+ {
+super("pair_left", arg);
+ }
+
+ @Override
+ public ExprEval eval(ObjectBinding bindings)
+ {
+Object pairResult = args.get(0).eval(bindings).value();
+if (pairResult instanceof SerializablePair){
+ return ExprEval.bestEffortOf(((SerializablePair) pairResult).lhs);
+}
+
+return ExprEval.of(null);
+ }
+
+ @Override
+ public Expr visit(Shuttle shuttle)
+ {
+return shuttle.visit(apply(shuttle.visitAll(args)));
+ }
+
+ @Nullable
+ @Override
+ public ExpressionType getOutputType(InputBindingInspector inspector)
+ {
+final ExpressionType inputType =
inspector.getType(arg.getBindingIfIdentifier());
Review Comment:
`inputType` could be `null` here. You could alternatively use
`arg.getOutputType(inspector)` though it can also still return null.
If the argument must be an identifier, i would recommend pulling
`arg.getBindingIfIdentifier()` and validating that it is not null separately.
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195: URL: https://github.com/apache/druid/pull/14195#discussion_r1295028080 ## sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java: ## @@ -109,7 +111,7 @@ AggregatorFactory createAggregatorFactory(String name, String fieldName, String return new DoubleLastAggregatorFactory(name, fieldName, timeColumn); case STRING: case COMPLEX: -return new StringLastAggregatorFactory(name, fieldName, timeColumn, maxStringBytes); +return new StringLastAggregatorFactory(name, fieldName, timeColumn, null, maxStringBytes, true); Review Comment: same comment about `COMPLEX` case and `foldColumn`. ## processing/src/main/java/org/apache/druid/query/aggregation/post/PostAggregatorIds.java: ## @@ -66,4 +66,5 @@ public class PostAggregatorIds public static final byte KLL_FLOATS_SKETCH_TO_QUANTILE_CACHE_TYPE_ID = 42; public static final byte KLL_FLOATS_SKETCH_TO_QUANTILES_CACHE_TYPE_ID = 43; public static final byte KLL_FLOATS_SKETCH_TO_STRING_CACHE_TYPE_ID = 44; + public static final byte PAIRS = 45; Review Comment: this isn't needed? -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295009168
##
processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstFoldingAggregator.java:
##
@@ -0,0 +1,90 @@
+/*
+ * 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.first;
+
+import org.apache.druid.java.util.common.DateTimes;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.query.aggregation.Aggregator;
+import org.apache.druid.query.aggregation.SerializablePairLongString;
+import org.apache.druid.segment.BaseObjectColumnValueSelector;
+
+public class StringFirstFoldingAggregator implements Aggregator
+{
+ private final BaseObjectColumnValueSelector
valueSelector;
+ private final int maxStringBytes;
+
+ protected long firstTime;
+ protected String firstValue;
+
+ public StringFirstFoldingAggregator(
+ BaseObjectColumnValueSelector pairSelector,
+ int maxStringBytes
+ )
+ {
+this.valueSelector = pairSelector;
+this.maxStringBytes = maxStringBytes;
+
+firstTime = DateTimes.MAX.getMillis();
+firstValue = null;
+ }
+
+ @Override
+ public void aggregate()
+ {
+// Less efficient code path when folding is a possibility (we must read
the value selector first just in case
+// it's a foldable object).
Review Comment:
nit: stale comment copied from old combined agg?
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295007928
##
processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstAggregatorFactory.java:
##
@@ -94,37 +110,67 @@ public void aggregate(ByteBuffer buf, int position)
Comparator.comparing(SerializablePair::getRhs,
Comparator.nullsFirst(Comparator.naturalOrder()))
);
- private final String fieldName;
private final String name;
+ private final String fieldName;
private final String timeColumn;
+ private final String foldColumn;
protected final int maxStringBytes;
+ private final boolean finalize;
@JsonCreator
public StringFirstAggregatorFactory(
@JsonProperty("name") String name,
- @JsonProperty("fieldName") final String fieldName,
+ @JsonProperty("fieldName") @Nullable final String fieldName,
@JsonProperty("timeColumn") @Nullable final String timeColumn,
- @JsonProperty("maxStringBytes") Integer maxStringBytes
+ @JsonProperty("foldColumn") @Nullable final String foldColumn,
Review Comment:
why do we need a separate `foldColumn`? It seems like instead we could just
not auto initialize the `timeColumn`, and in factorize methods examine the
`ColumnCapabilities` of `ColumnSelectorFactory` of the `fieldName` column and
factorize into either the folding aggregator or the building aggregator as
appropriate
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295035614
##
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/pair/EarliestLatestPairSqlAggregator.java:
##
@@ -0,0 +1,233 @@
+/*
+ * 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.sql.calcite.aggregation.builtin.pair;
+
+import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.type.InferTypes;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.util.Optionality;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.query.aggregation.AggregatorFactory;
+import org.apache.druid.query.aggregation.first.StringFirstAggregatorFactory;
+import org.apache.druid.query.aggregation.last.StringLastAggregatorFactory;
+import org.apache.druid.segment.column.ColumnType;
+import org.apache.druid.segment.column.RowSignature;
+import org.apache.druid.sql.calcite.aggregation.Aggregation;
+import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
+import org.apache.druid.sql.calcite.table.RowSignatures;
+
+import javax.annotation.Nullable;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+public class EarliestLatestPairSqlAggregator implements SqlAggregator
+{
+ public static final SqlAggregator[] DEFINED_AGGS = new SqlAggregator[]{
+ new EarliestLatestPairSqlAggregator(AggregatorType.EARLIEST,
PayloadType.STRING),
+ new EarliestLatestPairSqlAggregator(AggregatorType.LATEST,
PayloadType.STRING)
+ };
+
+ enum AggregatorType
+ {
+EARLIEST, LATEST
+ }
+
+ enum PayloadType
+ {
+STRING, LONG, DOUBLE
+ }
Review Comment:
do we really need new functions to do aggregations on complex types? can't
`EARLIEST` and `LATEST` detect when their inputs are a pair and do the right
thing?
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Add pair Latest/Earliest aggregators (druid)
clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295019822
##
processing/src/main/java/org/apache/druid/query/aggregation/pair/PairRightExprMacro.java:
##
@@ -0,0 +1,87 @@
+/*
+ * 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.pair;
+
+import org.apache.druid.collections.SerializablePair;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.math.expr.Expr;
+import org.apache.druid.math.expr.ExprEval;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.math.expr.ExprType;
+import org.apache.druid.math.expr.ExpressionType;
+import org.apache.druid.query.aggregation.first.StringFirstAggregatorFactory;
+
+import javax.annotation.Nullable;
+import java.util.List;
+
+public class PairRightExprMacro implements ExprMacroTable.ExprMacro
+{
+ @Override
+ public Expr apply(List args)
+ {
+validationHelperCheckArgumentCount(args, 1);
+Expr arg = args.get(0);
+
+class PairRightExpr extends
ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr
+{
+ public PairRightExpr(Expr arg)
+ {
+super("pair_right", arg);
+ }
+
+ @Override
+ public ExprEval eval(ObjectBinding bindings)
+ {
+Object pairResult = args.get(0).eval(bindings).value();
+if (pairResult instanceof SerializablePair){
+ return ExprEval.bestEffortOf(((SerializablePair) pairResult).rhs);
+}
+
+return ExprEval.of(null);
+ }
+
+ @Override
+ public Expr visit(Shuttle shuttle)
+ {
+return shuttle.visit(apply(shuttle.visitAll(args)));
+ }
+
+ @Nullable
+ @Override
+ public ExpressionType getOutputType(InputBindingInspector inspector)
+ {
+final ExpressionType inputType =
inspector.getType(arg.getBindingIfIdentifier());
Review Comment:
same comment about using `arg.getOutputType(inspector)`
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
