You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/10/13 01:02:12 UTC
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6066: Detect the behavior when column name mismatches in the query
jackjlli commented on a change in pull request #6066:
URL: https://github.com/apache/incubator-pinot/pull/6066#discussion_r503610423
##########
File path: pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -769,94 +786,133 @@ private void updateColumnNames(BrokerRequest brokerRequest) {
for (int i = 0; i < selectionColumns.size(); i++) {
String expression = selectionColumns.get(i);
if (!expression.equals("*")) {
- selectionColumns.set(i, fixColumnName(rawTableName, expression, columnNameMap));
+ selectionColumns.set(i, fixColumnName(rawTableName, expression, columnNameMap, query));
}
}
}
if (brokerRequest.isSetOrderBy()) {
List<SelectionSort> orderBy = brokerRequest.getOrderBy();
for (SelectionSort selectionSort : orderBy) {
String expression = selectionSort.getColumn();
- selectionSort.setColumn(fixColumnName(rawTableName, expression, columnNameMap));
+ selectionSort.setColumn(fixColumnName(rawTableName, expression, columnNameMap, query));
}
}
PinotQuery pinotQuery = brokerRequest.getPinotQuery();
if (pinotQuery != null) {
for (Expression expression : pinotQuery.getSelectList()) {
- fixColumnName(rawTableName, expression, columnNameMap);
+ fixColumnName(rawTableName, expression, columnNameMap, query);
}
Expression filterExpression = pinotQuery.getFilterExpression();
if (filterExpression != null) {
- fixColumnName(rawTableName, filterExpression, columnNameMap);
+ fixColumnName(rawTableName, filterExpression, columnNameMap, query);
}
List<Expression> groupByList = pinotQuery.getGroupByList();
if (groupByList != null) {
for (Expression expression : groupByList) {
- fixColumnName(rawTableName, expression, columnNameMap);
+ fixColumnName(rawTableName, expression, columnNameMap, query);
}
}
List<Expression> orderByList = pinotQuery.getOrderByList();
if (orderByList != null) {
for (Expression expression : orderByList) {
- fixColumnName(rawTableName, expression, columnNameMap);
+ fixColumnName(rawTableName, expression, columnNameMap, query);
}
}
Expression havingExpression = pinotQuery.getHavingExpression();
if (havingExpression != null) {
- fixColumnName(rawTableName, havingExpression, columnNameMap);
+ fixColumnName(rawTableName, havingExpression, columnNameMap, query);
}
}
}
- private String fixColumnName(String rawTableName, String expression, @Nullable Map<String, String> columnNameMap) {
+ private String fixColumnName(String rawTableName, String expression, @Nullable Map<String, String> columnNameMap,
+ String query) {
TransformExpressionTree expressionTree = TransformExpressionTree.compileToExpressionTree(expression);
- fixColumnName(rawTableName, expressionTree, columnNameMap);
+ fixColumnName(rawTableName, expressionTree, columnNameMap, query);
return expressionTree.toString();
}
private void fixColumnName(String rawTableName, TransformExpressionTree expression,
- @Nullable Map<String, String> columnNameMap) {
+ @Nullable Map<String, String> columnNameMap, String query) {
TransformExpressionTree.ExpressionType expressionType = expression.getExpressionType();
if (expressionType == TransformExpressionTree.ExpressionType.IDENTIFIER) {
- expression.setValue(getActualColumnName(rawTableName, expression.getValue(), columnNameMap));
+ expression.setValue(
+ getActualColumnName(rawTableName, expression.getValue(), columnNameMap, query));
} else if (expressionType == TransformExpressionTree.ExpressionType.FUNCTION) {
for (TransformExpressionTree child : expression.getChildren()) {
- fixColumnName(rawTableName, child, columnNameMap);
+ fixColumnName(rawTableName, child, columnNameMap, query);
}
}
}
- private void fixColumnName(String rawTableName, Expression expression, @Nullable Map<String, String> columnNameMap) {
+ private void fixColumnName(String rawTableName, Expression expression, @Nullable Map<String, String> columnNameMap,
+ String query) {
ExpressionType expressionType = expression.getType();
if (expressionType == ExpressionType.IDENTIFIER) {
- Identifier identifier = expression.getIdentifier();
- identifier.setName(getActualColumnName(rawTableName, identifier.getName(), columnNameMap));
+ if (!isStarIdentifier(expression)) {
+ Identifier identifier = expression.getIdentifier();
+ identifier.setName(getActualColumnName(rawTableName, identifier.getName(), columnNameMap,
+ query));
+ }
} else if (expressionType == ExpressionType.FUNCTION) {
- for (Expression operand : expression.getFunctionCall().getOperands()) {
- fixColumnName(rawTableName, operand, columnNameMap);
+ String operator = expression.getFunctionCall().getOperator();
+ List<Expression> expressions = expression.getFunctionCall().getOperands();
+ if ("AS".equals(operator)) {
+ fixColumnName(rawTableName, expressions.get(0), columnNameMap, query);
+ } else if (!isCountStarFromExpression(expression)) {
+ for (Expression operand : expression.getFunctionCall().getOperands()) {
+ fixColumnName(rawTableName, operand, columnNameMap, query);
+ }
}
}
}
- private String getActualColumnName(String rawTableName, String columnName,
- @Nullable Map<String, String> columnNameMap) {
+ private boolean isCountStarFromExpression(Expression expression) {
+ String operator = expression.getFunctionCall().getOperator();
+ List<Expression> expressions = expression.getFunctionCall().getOperands();
+ return "COUNT".equals(operator) && expressions.size() == 1 && isStarIdentifier(expressions.get(0));
+ }
+
+ private boolean isStarIdentifier(Expression expression) {
+ return "*".equals(expression.getIdentifier().getName());
+ }
+
+ private String getActualColumnName(String rawTableName, String columnName, Map<String, String> columnNameMap,
+ String query) {
// Check if column is in the format of [table_name].[column_name]
String[] splits = StringUtils.split(columnName, ".", 2);
if (_tableCache.isCaseInsensitive()) {
if (splits.length == 2 && rawTableName.equalsIgnoreCase(splits[0])) {
columnName = splits[1];
}
- if (columnNameMap != null) {
- return columnNameMap.getOrDefault(columnName, columnName);
+ if (columnNameMap == null) {
+ return columnName;
+ }
+ String actualColumnName = columnNameMap.get(columnName.toLowerCase());
+ if (actualColumnName != null) {
+ return actualColumnName;
+ } else if (_enableFailQueryOnColumnMismatch) {
+ throw new BadQueryRequestException("Invalid column name: " + columnName + " in the query: " + query);
} else {
+ LOGGER.warn("Invalid column name: {} in the query: {}", columnName, query);
Review comment:
Removed this message.
----------------------------------------------------------------
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