Jackie-Jiang commented on a change in pull request #7590:
URL: https://github.com/apache/pinot/pull/7590#discussion_r736953973
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1396,14 +1411,34 @@ private void fixColumnName(String rawTableName,
TransformExpressionTree expressi
/**
* Fixes the column names to the actual column names in the given SQL
expression.
*/
- private void fixColumnName(String rawTableName, Expression expression,
@Nullable Map<String, String> columnNameMap) {
+ @VisibleForTesting
Review comment:
(minor) Annotated the wrong method. Should be for the method on line 1298
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1413,25 +1448,43 @@ private void fixColumnName(String rawTableName,
Expression expression, @Nullable
* - Case-insensitive cluster
* - Column name in the format of [table_name].[column_name]
*/
- private String getActualColumnName(String rawTableName, String columnName,
- @Nullable Map<String, String> columnNameMap) {
+ private static String getActualColumnName(String rawTableName, String
columnName,
+ @Nullable Map<String, String> columnNameMap, @Nullable Map<String,
String> aliasMap, boolean isCaseInsensitive) {
+ if ("*".equals(columnName)) {
+ return columnName;
+ }
// Check if column is in the format of [table_name].[column_name]
String[] splits = StringUtils.split(columnName, ".", 2);
- if (_tableCache.isCaseInsensitive()) {
+ if (isCaseInsensitive) {
if (splits.length == 2 && rawTableName.equalsIgnoreCase(splits[0])) {
columnName = splits[1];
}
+ String columnNameLowerCase = columnName.toLowerCase();
if (columnNameMap != null) {
- return columnNameMap.getOrDefault(columnName.toLowerCase(),
columnName);
- } else {
- return columnName;
+ String actualColumnName = columnNameMap.get(columnNameLowerCase);
+ if (actualColumnName != null) {
+ return actualColumnName;
+ }
+ }
+ if (aliasMap != null) {
+ String actualAlias = aliasMap.get(columnNameLowerCase);
+ if (actualAlias != null) {
+ return actualAlias;
+ }
}
} else {
if (splits.length == 2 && rawTableName.equals(splits[0])) {
columnName = splits[1];
}
- return columnName;
+ String columnNameLowerCase = columnName.toLowerCase();
Review comment:
We should not use lower case when it is case sensitive.
We can use the same logic for them as followings:
```suggestion
String columnNameToCheck;
if (isCaseInsensitive) {
if (splits.length == 2 && rawTableName.equalsIgnoreCase(splits[0])) {
columnNameToCheck = splits[1].toLowerCase();
} else {
columnNameToCheck = columnName.toLowerCase();
}
} else {
if (splits.length == 2 && rawTableName.equals(splits[0])) {
columnNameToCheck = splits[1];
} else {
columnNameToCheck = columnName;
}
}
if (columnNameMap != null) {
String actualColumnName = columnNameMap.get(columnNameToCheck);
if (actualColumnName != null) {
return actualColumnName;
}
}
if (aliasMap != null) {
String actualAlias = aliasMap.get(columnNameToCheck);
if (actualAlias != null) {
return actualAlias;
}
}
throw new BadQueryRequestException("Unknown columnName '" + columnName +
"' found in the query");
```
--
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.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
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
