hudi-agent commented on code in PR #18726:
URL: https://github.com/apache/hudi/pull/18726#discussion_r3831947233


##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/analysis/HoodieAnalysis.scala:
##########
@@ -639,3 +643,70 @@ case class HoodiePostAnalysisRule(sparkSession: 
SparkSession) extends Rule[Logic
     }
   }
 }
+
+/**
+ * Stamps a synthesized [[CatalogTable]] (table name, base path, schema) onto 
path-based
+ * Hudi reads whose underlying file index is incremental or CDC. Without it, 
lineage and
+ * governance tooling sees `LogicalRelation.catalogTable = None` and falls 
back to the
+ * relation's class name as the dataset identifier -- useless for tracking 
which table
+ * an incremental query came from.
+ *
+ * Scope is intentionally limited to incremental and CDC reads:
+ *  - Catalog-registered reads already have `catalogTable` populated.
+ *  - Path-based snapshot reads have a working file-path-based fallback in 
existing
+ *    lineage tooling; changing their behavior is a separate decision.
+ */
+object HoodieIncrementalRelationIdentifier extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan =
+    AnalysisHelper.allowInvokingTransformsInAnalyzer {
+      plan transform {
+        // Type pattern + nested match avoids destructuring `LogicalRelation`, 
whose
+        // case-class arity differs between Spark 3.x (4 args) and Spark 4.x 
(5 args). This
+        // rule lives in `hudi-spark`, which is compiled against every 
supported profile.
+        case lr: LogicalRelation if lr.catalogTable.isEmpty =>
+          lr.relation match {
+            case fsRelation: HadoopFsRelation =>

Review Comment:
   🤖 nit: the lambda parameter `catalogTable` shadows the field name 
`catalogTable` in `lr.copy(catalogTable = Some(catalogTable))`, so parsing the 
expression requires a double-take. Could you rename the parameter to something 
like `ct` or `table` to make the chain scan a bit faster?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/analysis/HoodieAnalysis.scala:
##########
@@ -639,3 +643,70 @@ case class HoodiePostAnalysisRule(sparkSession: 
SparkSession) extends Rule[Logic
     }
   }
 }
+
+/**
+ * Stamps a synthesized [[CatalogTable]] (table name, base path, schema) onto 
path-based
+ * Hudi reads whose underlying file index is incremental or CDC. Without it, 
lineage and
+ * governance tooling sees `LogicalRelation.catalogTable = None` and falls 
back to the
+ * relation's class name as the dataset identifier -- useless for tracking 
which table
+ * an incremental query came from.
+ *
+ * Scope is intentionally limited to incremental and CDC reads:
+ *  - Catalog-registered reads already have `catalogTable` populated.
+ *  - Path-based snapshot reads have a working file-path-based fallback in 
existing
+ *    lineage tooling; changing their behavior is a separate decision.
+ */
+object HoodieIncrementalRelationIdentifier extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan =
+    AnalysisHelper.allowInvokingTransformsInAnalyzer {
+      plan transform {
+        // Type pattern + nested match avoids destructuring `LogicalRelation`, 
whose
+        // case-class arity differs between Spark 3.x (4 args) and Spark 4.x 
(5 args). This
+        // rule lives in `hudi-spark`, which is compiled against every 
supported profile.
+        case lr: LogicalRelation if lr.catalogTable.isEmpty =>
+          lr.relation match {
+            case fsRelation: HadoopFsRelation =>
+              incrementalOrCDCIndex(fsRelation.location)
+                .flatMap(index => buildCatalogTable(index.metaClient, 
lr.schema))
+                .map(catalogTable => lr.copy(catalogTable = 
Some(catalogTable)))
+                .getOrElse(lr)
+            case _ => lr
+          }
+      }
+    }
+
+  /**

Review Comment:
   🤖 nit: the phrase "the widening is checked by the compiler rather than by an 
`asInstanceOf`" is a bit hard to parse -- `asInstanceOf` is a cast, not a 
check, so the comparison is a little off. Could you rephrase to something like: 
"Typed as `Option[HoodieFileIndex]` so the compiler verifies both matched types 
are subtypes -- a future third index added without updating this method won't 
accidentally slip through as `None`"?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



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