Jackie-Jiang commented on a change in pull request #7590:
URL: https://github.com/apache/pinot/pull/7590#discussion_r731378330



##########
File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -247,10 +247,12 @@ private BrokerResponseNative handleSQLRequest(long 
requestId, String query, Json
     requestStatistics.setTableName(rawTableName);
 
     try {
-      updateColumnNames(rawTableName, pinotQuery);
+      boolean isCaseInsensitive = _tableCache.isCaseInsensitive();
+      Map<String, String> columnNameMap = isCaseInsensitive ? 
_tableCache.getColumnNameMap(rawTableName) : null;
+      updateColumnNames(rawTableName, pinotQuery, isCaseInsensitive, 
columnNameMap);
     } catch (Exception e) {

Review comment:
       Catch `BadQueryRequestException` and create an error response

##########
File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1283,33 +1285,33 @@ private void computeResultsForLiteral(Literal literal, 
List<String> columnNames,
   /**
    * Fixes the column names to the actual column names in the given SQL query.
    */
-  private void updateColumnNames(String rawTableName, PinotQuery pinotQuery) {
-    Map<String, String> columnNameMap =
-        _tableCache.isCaseInsensitive() ? 
_tableCache.getColumnNameMap(rawTableName) : null;
+  static void updateColumnNames(String rawTableName, PinotQuery pinotQuery, 
boolean isCaseInsensitive,

Review comment:
       (nit) Annotate with `VisibleForTesting`

##########
File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1397,14 +1400,29 @@ 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) {
+  private static void fixColumnName(String rawTableName, Expression expression,
+      @Nullable Map<String, String> columnNameMap, boolean isCaseInsensitive) {

Review comment:
       `columnNameMap` cannot be `null`




-- 
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

Reply via email to