zhangbutao commented on code in PR #6363:
URL: https://github.com/apache/hive/pull/6363#discussion_r2937888163
##########
iceberg/iceberg-handler/src/test/queries/positive/iceberg_branch_with_table.q:
##########
@@ -0,0 +1,43 @@
+CREATE EXTERNAL TABLE default.hive_branch_bug
+( foo BIGINT )
+STORED BY ICEBERG;
+
+ALTER TABLE default.hive_branch_bug CREATE BRANCH empty;
+
+INSERT INTO default.hive_branch_bug (foo) VALUES (1), (2), (3), (4);
+
+SELECT COUNT(*)
+FROM default.hive_branch_bug
+UNION ALL
+SELECT COUNT(*)
+FROM default.hive_branch_bug.branch_empty;
+
+SELECT COUNT(*)
+FROM default.hive_branch_bug.branch_empty
+UNION ALL
+SELECT COUNT(*)
+FROM default.hive_branch_bug;
+
+SELECT
+(SELECT COUNT(*) FROM default.hive_branch_bug),
+(SELECT COUNT(*) FROM default.hive_branch_bug.branch_empty);
+
+ SELECT COUNT(*)
+FROM default.hive_branch_bug
+WHERE foo > 0
+UNION ALL
+SELECT COUNT(*)
+FROM default.hive_branch_bug.branch_empty
+WHERE foo > 0;
+
+SELECT *
+FROM (
+SELECT COUNT(*) c
+FROM default.hive_branch_bug
+) a
+UNION ALL
+SELECT *
+FROM (
+SELECT COUNT(*) c
+FROM default.hive_branch_bug.branch_empty
+) b;
Review Comment:
Nit: Add line breaks.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedWorkOptimizer.java:
##########
@@ -645,6 +645,11 @@ protected boolean areMergeable(ParseContext pctx,
TableScanOperator tsOp1, Table
LOG.debug("opProps differ {} ~ {}", tsOp1.getConf().getOpProps(),
tsOp2.getConf().getOpProps());
return false;
}
+
Review Comment:
Suggested to add a comment here:
`// HIVE-29509: Include snapshotRef to ensure different Iceberg
branches/tags are treated as distinct tables`
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java:
##########
@@ -606,6 +603,9 @@ public boolean equals(Object obj) {
if (!Objects.equals(versionIntervalFrom, other.versionIntervalFrom)) {
return false;
}
+ if (!Objects.equals(snapshotRef, other.snapshotRef)) {
Review Comment:
Nit: Current code (lines 590-609): Manual null check is verbose
Suggested optimization:
```
return Objects.equals(tTable, other.tTable)
&& Objects.equals(asOfTimestamp, other.asOfTimestamp)
&& Objects.equals(asOfVersion, other.asOfVersion)
&& Objects.equals(versionIntervalFrom, other.versionIntervalFrom)
&& Objects.equals(snapshotRef, other.snapshotRef);
```
--
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]
