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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]