[HUDI-8432] Fix data skipping with RLI if record key is composite #12160
Conversation
| if (attribute != null && attribute.name != null && attributeMatchesRecordKey(attribute.name, recordKeyOpt)) { | ||
| Option.apply(equalToQuery, List.apply(literal.value.toString)) | ||
| if (attribute != null && attribute.name != null && recordKeyOpt.contains(attribute.name)) { | ||
| val recordKeyLiteral = getRecordKeyLiteral(attribute, literal, isComplexRecordKey) |
There was a problem hiding this comment.
How can we ensure all the consitituent record key values got involved in the expression?
There was a problem hiding this comment.
good point.. we should check if the predicate contains all of record key fields. Technically, we can support subset of record key fields as well but that involves pulling all records in RLI and checking for subset match. Something to followup. For now, we can just see if all record key fields are present or not. If not, then don't use RLI. What do you think?
There was a problem hiding this comment.
For now, we can just see if all record key fields are present or not. If not, then don't use RLI. What do you think?
I kind of think we should also restrain all the expression type to EQUALS. For e.g, we have record key col_0,col_1 and expression col_0 >0 && col_1 <100, you can not utilitize RLI in sucn use case.
There was a problem hiding this comment.
yes.
- lets do RLI look up only when all fields in complex key gen are part of the predicates.
- If I am not wrong, we already only support "IN" and "EQUAL"s. So, not sure what Danny is bringing up.
There was a problem hiding this comment.
We don't support range queries with RLI, so it will fallback to no data skipping (exxcept for partition pruning which is done before we read RLI).
There was a problem hiding this comment.
We need to check all the record key fields are got involved.
codope
force-pushed
the
hudi-8432-rec-index-complex
branch
from
2ecc9e7
to
b20398a
Compare
| } | ||
|
|
||
| // Handle Or expression (for completeness, though usually not used for record key filtering) | ||
| case orQuery: Or => |
There was a problem hiding this comment.
I guess the OR does not work as expected.
| validINQuery = false | ||
| } | ||
| case _ => validINQuery = false | ||
| } | ||
| var literals: List[String] = List.empty | ||
| inQuery.list.foreach { | ||
| case literal: Literal => literals = literals :+ literal.value.toString | ||
| case literal: Literal => literals = literals :+ getRecordKeyLiteral(inQuery.value.asInstanceOf[AttributeReference], literal, isComplexRecordKey) |
There was a problem hiding this comment.
We need to check all the record key fields are got involved.
| })) | ||
| Option.apply(recordKeyOpt.orElse(null)) | ||
| // Convert the Hudi Option to Scala Option and return if present | ||
| Option(recordKeysOpt.orElse(null)).filter(_.nonEmpty) |
There was a problem hiding this comment.
The nonEmpty does now work when the record key is null.
| verifyEqualToQuery(hudiOpts, Seq("record_key_col", "name"), tableName, latestSnapshotDf, HoodieTableMetaClient.reload(metaClient)) | ||
| } | ||
|
|
||
| def verifyEqualToQuery(hudiOpts: Map[String, String], colNames: Seq[String], tableName: String, latestSnapshotDf: DataFrame, metaClient: HoodieTableMetaClient): Unit = { |
Change Logs
Fix data skipping with RLI if record key is composite. When there are multiple record key fields, we need to form the record keys sequence correctly. Added a test to show skipping with more than one record key fields.
Impact
Support data skipping for complex primary keys
Risk level (write none, low medium or high below)
low
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist