walterddr commented on code in PR #11956:
URL: https://github.com/apache/pinot/pull/11956#discussion_r1385174026
##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/RequestContextUtils.java:
##########
@@ -111,11 +111,9 @@ public static FilterContext getFilter(Expression
thriftExpression) {
return getFilter(thriftFunction);
case IDENTIFIER:
// Convert "WHERE a" to "WHERE a = true"
- return new FilterContext(FilterContext.Type.PREDICATE, null,
- new EqPredicate(getExpression(thriftExpression),
getStringValue(RequestUtils.getLiteralExpression(true))));
+ return FilterContext.forPredicate(new
EqPredicate(getExpression(thriftExpression), "true"));
case LITERAL:
- // TODO: Handle literals.
- throw new IllegalStateException();
+ return FilterContext.forConstant(new
LiteralContext(thriftExpression.getLiteral()).getBooleanValue());
Review Comment:
general question: why do we need to repeat ourselves on these utils one for
thrift and one for function context?
it looks like the only difference is one is for WHERE and one is for HAVING?
##########
pinot-core/src/test/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverterTest.java:
##########
@@ -658,7 +658,7 @@ public void testFilteredAggregations() {
}
@Test
- void testDeduplicateOrderByExpressions() {
+ public void testDeduplicateOrderByExpressions() {
Review Comment:
LOL
--
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
