[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6808: [PQL Deprecation] Remove the usage of PQL compiler

[GitBox <gi...@apache.org>]

fx19880617 commented on a change in pull request #6808:
URL: https://github.com/apache/incubator-pinot/pull/6808#discussion_r616240012



##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunctionTest.java
##########
@@ -112,25 +110,13 @@ private void testCaseQueryWithDoubleResults(String predicate, double[] expectedV
 
   private void testCaseQueryWithStringResults(String predicate, String[] expectedValues) {
     ExpressionContext expression =
-        RequestContextUtils.getExpression(String.format("CASE(%s, 'aaa', 'bbb')", predicate));
+        RequestContextUtils.getExpressionFromSQL(String.format("CASE WHEN %s THEN 'aaa' ELSE 'bbb' END", predicate));
     TransformFunction transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
     Assert.assertTrue(transformFunction instanceof CaseTransformFunction);
     Assert.assertEquals(transformFunction.getName(), CaseTransformFunction.FUNCTION_NAME);
     testTransformFunction(transformFunction, expectedValues);
   }
 
-  @Test(dataProvider = "testIllegalArguments", expectedExceptions = {BadQueryRequestException.class})

Review comment:
       why we removed these tests?

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java
##########
@@ -141,21 +141,31 @@ private SelectionOperatorUtils() {
    * Expands {@code 'SELECT *'} to all columns (excluding transform functions) within {@link DataSchema} with
    * alphabetical order if applies.
    */
-  public static List<String> getSelectionColumns(List<ExpressionContext> selectExpressions, DataSchema dataSchema) {
+  public static List<String> getSelectionColumns(QueryContext queryContext, DataSchema dataSchema) {
+    List<ExpressionContext> selectExpressions = queryContext.getSelectExpressions();
     int numSelectExpressions = selectExpressions.size();
     if (numSelectExpressions == 1 && selectExpressions.get(0).equals(IDENTIFIER_STAR)) {
       String[] columnNames = dataSchema.getColumnNames();
       int numColumns = columnNames.length;
 
-      // Note: The data schema might be generated from DataTableBuilder.buildEmptyDataTable(), where for 'SELECT *' it
+      // NOTE: The data schema might be generated from DataTableBuilder.buildEmptyDataTable(), where for 'SELECT *' it
       //       contains a single column "*". In such case, return as is to build the empty selection result.
       if (numColumns == 1 && columnNames[0].equals("*")) {
         return Collections.singletonList("*");
       }
 
+      // Directly return all columns for selection-only queries
+      // NOTE: Order-by expressions are ignored for queries with LIMIT 0
+      List<OrderByExpressionContext> orderByExpressions = queryContext.getOrderByExpressions();
+      if (orderByExpressions == null || queryContext.getLimit() == 0) {
+        return Arrays.asList(columnNames);
+      }
+
+      // Exclude transform functions from the returned columns and sort
+      // NOTE: Do not parse the column because it might contain SQL reserved words
       List<String> allColumns = new ArrayList<>(numColumns);
       for (String column : columnNames) {
-        if (RequestContextUtils.getExpression(column).getType() == ExpressionContext.Type.IDENTIFIER) {
+        if (column.indexOf('(') == -1) {

Review comment:
       hmm, why literals are allowed here?




-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org