Jackie-Jiang commented on code in PR #9184:
URL: https://github.com/apache/pinot/pull/9184#discussion_r948437161
##########
pinot-common/src/thrift/query.thrift:
##########
@@ -70,4 +70,4 @@ union Literal {
struct Function {
1: required string operator;
2: optional list<Expression> operands;
-}
+}
Review Comment:
(minor) revert
##########
pinot-common/src/main/java/org/apache/pinot/common/request/Expression.java:
##########
@@ -38,7 +38,7 @@ public class Expression implements
org.apache.thrift.TBase<Expression, Expressio
private static final org.apache.thrift.scheme.SchemeFactory
TUPLE_SCHEME_FACTORY = new ExpressionTupleSchemeFactory();
/**
- *
+ *
Review Comment:
(minor) Revert the changes in this file
##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/PinotClientTransport.java:
##########
@@ -42,4 +42,4 @@ Future<BrokerResponse> executeQueryAsync(String
brokerAddress, Request request)
void close()
throws PinotClientException;
-}
+}
Review Comment:
(minor) Please revert this
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -956,4 +985,4 @@ public static boolean isLiteralOnlyExpression(Expression e)
{
}
return false;
}
-}
+}
Review Comment:
(minor) revert
##########
pinot-common/src/main/java/org/apache/pinot/common/request/ExpressionType.java:
##########
@@ -49,7 +49,7 @@ public int getValue() {
* @return null if the value is not found.
*/
@org.apache.thrift.annotation.Nullable
- public static ExpressionType findByValue(int value) {
+ public static ExpressionType findByValue(int value) {
Review Comment:
(minor) Revert the changes in this file
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -752,6 +754,8 @@ private static Expression
compileFunctionExpression(SqlBasicCall functionNode) {
case OR:
return compileOrExpression(functionNode);
// BETWEEN and LIKE might be negated (NOT BETWEEN, NOT LIKE)
+ case EXTRACT:
Review Comment:
Do we need to handle this separately? Seems it is the same as the default
handling logic
##########
pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java:
##########
@@ -96,6 +96,8 @@ public enum TransformFunctionType {
LOOKUP("lookUp"),
GROOVY("groovy"),
+ EXTRACT_YEAR("extractYear"),
Review Comment:
We want to add one `EXTRACT` transform function for all time unit, instead
of one function for each unit
##########
pinot-common/src/main/java/org/apache/pinot/sql/FilterKind.java:
##########
@@ -23,6 +23,7 @@ public enum FilterKind {
OR,
NOT,
EQUALS,
+ EXTRACT,
Review Comment:
This shouldn't be a filter kind
--
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...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
