mcvsubbu commented on a change in pull request #6192:
URL: https://github.com/apache/incubator-pinot/pull/6192#discussion_r513817431
##########
File path:
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ClusterIntegrationTestUtils.java
##########
@@ -859,7 +859,9 @@ static void testSqlQuery(String pinotQuery, String
brokerUrl, org.apache.pinot.c
BrokerRequest brokerRequest =
PinotQueryParserFactory.get(CommonConstants.Broker.Request.SQL).compileToBrokerRequest(pinotQuery);
if (isSelectionQuery(brokerRequest)) { // selection
- // TODO: compare results for selection queries, w/o order by
+ // TODO: 1. compare results for selection queries, w/o order by.
+ // 2. validate values of multi-value columns.
+ // 3. remove the restriction that order by column has to be in
selection clause.
Review comment:
Can you add the issue number here? thanks
##########
File path:
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java
##########
@@ -478,7 +479,8 @@ protected void stopKafka() {
*/
protected long getCurrentCountStarResult()
throws Exception {
- return getPinotConnection().execute("SELECT COUNT(*) FROM " +
getTableName()).getResultSet(0).getLong(0);
+ return getPinotConnection().execute(new Request("pql", "SELECT COUNT(*)
FROM " + getTableName())).getResultSet(0)
Review comment:
Why not make this "sql"? We will phase out pql anyway, so we can start
doing this via sql all ths 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.
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
