bhat-vinay commented on code in PR #10947:
URL: https://github.com/apache/hudi/pull/10947#discussion_r1551485041


##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/RecordLevelIndexSupport.scala:
##########
@@ -180,9 +157,40 @@ class RecordLevelIndexSupport(spark: SparkSession,
   }
 
   /**
-   * Return true if metadata table is enabled and record index metadata 
partition is available.
+   * Returns the attribute and literal pair given the operands of a binary 
operator. The pair is returned only if one of
+   * the operand is an attribute and other is literal. In other cases it 
returns an empty Option.
+   * @param expression1 - Left operand of the binary operator
+   * @param expression2 - Right operand of the binary operator
+   * @return Attribute and literal pair
    */
-  def isIndexAvailable: Boolean = {
-    metadataConfig.enabled && 
metaClient.getTableConfig.getMetadataPartitions.contains(HoodieTableMetadataUtil.PARTITION_NAME_RECORD_INDEX)
+  private def getAttributeLiteralTuple(expression1: Expression, expression2: 
Expression): Option[(AttributeReference, Literal)] = {
+    expression1 match {
+      case attr: AttributeReference => expression2 match {
+        case literal: Literal =>
+          Option.apply(attr, literal)
+        case _ =>
+          Option.empty
+      }
+      case literal: Literal => expression2 match {
+        case attr: AttributeReference =>
+          Option.apply(attr, literal)
+        case _ =>
+          Option.empty
+      }
+      case _ => Option.empty
+    }
+  }
+
+  /**
+   * Matches the configured simple record key with the input attribute name.
+   * @param attributeName The attribute name provided in the query
+   * @return true if input attribute name matches the configured simple record 
key
+   */
+  private def attributeMatchesRecordKey(attributeName: String, recordKeyOpt: 
Option[String]): Boolean = {

Review Comment:
   Refactoring for better unit testability. `getRecordKeyConfig()` touched 
instance member of `RecordLevelIndexSupport`



##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/RecordLevelIndexSupport.scala:
##########
@@ -143,22 +104,38 @@ class RecordLevelIndexSupport(spark: SparkSession,
     }
   }
 
+  /**
+   * Return true if metadata table is enabled and record index metadata 
partition is available.
+   */
+  def isIndexAvailable: Boolean = {
+    metadataConfig.enabled && 
metaClient.getTableConfig.getMetadataPartitions.contains(HoodieTableMetadataUtil.PARTITION_NAME_RECORD_INDEX)
+  }
+}
+
+object RecordLevelIndexSupport {
   /**
    * If the input query is an EqualTo or IN query on simple record key 
columns, the function returns a tuple of
    * list of the query and list of record key literals present in the query 
otherwise returns an empty option.
    *
    * @param queryFilter The query that need to be filtered.
    * @return Tuple of filtered query and list of record key literals that need 
to be matched
    */
-  private def filterQueryWithRecordKey(queryFilter: Expression): 
Option[(Expression, List[String])] = {
+  def filterQueryWithRecordKey(queryFilter: Expression, recordKeyOpt: 
Option[String]): Option[(Expression, List[String])] = {
     queryFilter match {
       case equalToQuery: EqualTo =>
-        val (attribute, literal) = getAttributeLiteralTuple(equalToQuery.left, 
equalToQuery.right).orNull
-        if (attribute != null && attribute.name != null && 
attributeMatchesRecordKey(attribute.name)) {
-          Option.apply(equalToQuery, List.apply(literal.value.toString))
+        val attributeLiteralTuple = 
getAttributeLiteralTuple(equalToQuery.left, equalToQuery.right).orNull
+        if (attributeLiteralTuple != null) {

Review Comment:
   A query of the form `select * from t1 where from_unixtime(ts) = 
'2020-01-01'`. Case 5 in the added unit tests.



##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/RecordLevelIndexSupport.scala:
##########
@@ -180,9 +157,40 @@ class RecordLevelIndexSupport(spark: SparkSession,
   }
 
   /**
-   * Return true if metadata table is enabled and record index metadata 
partition is available.
+   * Returns the attribute and literal pair given the operands of a binary 
operator. The pair is returned only if one of
+   * the operand is an attribute and other is literal. In other cases it 
returns an empty Option.
+   * @param expression1 - Left operand of the binary operator
+   * @param expression2 - Right operand of the binary operator
+   * @return Attribute and literal pair
    */
-  def isIndexAvailable: Boolean = {
-    metadataConfig.enabled && 
metaClient.getTableConfig.getMetadataPartitions.contains(HoodieTableMetadataUtil.PARTITION_NAME_RECORD_INDEX)
+  private def getAttributeLiteralTuple(expression1: Expression, expression2: 
Expression): Option[(AttributeReference, Literal)] = {

Review Comment:
   It is tested as part of unit testing `filterQueryWithRecordKey(...)` [added 
in `TestRecordLevelIndexSupport.scala`]



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

Reply via email to