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



##########
File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1614,40 +1614,48 @@ private static void fixColumnName(String rawTableName, 
Expression expression, Ma
    * - Case-insensitive cluster
    * - Column name in the format of [table_name].[column_name]
    */
-  private static String getActualColumnName(String rawTableName, String 
columnName,
+  private static String getActualColumnName(String rawTableName, String 
sqlColumnName,

Review comment:
       Let's not change the argument name here. If we want to preserve the name 
for exception message, we can either use a separate variable to store the 
mutated column name like the current code, or keep it in a separate local 
variable `String originalColumnName = columnName;`

##########
File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1614,40 +1614,48 @@ private static void fixColumnName(String rawTableName, 
Expression expression, Ma
    * - Case-insensitive cluster
    * - Column name in the format of [table_name].[column_name]
    */
-  private static String getActualColumnName(String rawTableName, String 
columnName,
+  private static String getActualColumnName(String rawTableName, String 
sqlColumnName,
       @Nullable Map<String, String> columnNameMap, @Nullable Map<String, 
String> aliasMap, boolean isCaseInsensitive) {
-    if ("*".equals(columnName)) {
-      return columnName;
+    if ("*".equals(sqlColumnName)) {
+      return sqlColumnName;
     }
-    // Check if column is in the format of [table_name].[column_name]
-    String[] splits = StringUtils.split(columnName, ".", 2);
-    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;
+
+    String columnName = sqlColumnName;
+    // If first part is a table name, then treat the second part as column 
name; otherwise, treat the entire identifier
+    // as column name.
+    String[] tableSplit = StringUtils.split(columnName, ".", 2);
+    if (tableSplit.length == 2) {
+      if ((isCaseInsensitive && rawTableName.equalsIgnoreCase(tableSplit[0])) 
|| rawTableName.equals(tableSplit[0])) {
+        columnName = tableSplit[1];
       }
     }
+
+    // Split again to check if the column name is suffixed by a path 
expression.
+    String[] columnSplit = StringUtils.split(columnName, ".", 2);

Review comment:
       This won't work if the column name itself contains `.`. We should first 
check if the column exist, if not, try to treat it as json path expression.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to