Jackie-Jiang commented on a change in pull request #6808:
URL: https://github.com/apache/incubator-pinot/pull/6808#discussion_r616338419
##########
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:
As described in the comments on line 165, if the column contains SQL
reserved word, it won't parse properly.
With the current code (PQL parser, see `Pql2Compiler.parseToAstNode()`), we
treat reserved words as identifier, which won't be correct if the column is an
expression with reserver words (e.g. `sum(time, 1)`)
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
