Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24577 )
Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature. ...................................................................... Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java: http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152 PS15, Line 152: String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode(); > I think we can keep this simpler. Generate the synthetic name once in `Impa Ok, I don't mind using a stable hash. Is there one in specific you would recommend? Never used one before. But before I do this, I want to code something you would approve of. So I did a search and came up with the HashFunction in Guava, which I know we link against. Is this what you're thinking? I don't want to include another library. And I need some hash code creator because the string needs to be stable, compact, and without special characters. So this doesn't look super straightforward, since I really only have objects that also have to use this special Hash fucnction. I suppose I can somehow use the string from the "Expr asOfExpr.toString()" passed into the constructor along with the Kind? I'd somehow have to use that Funnel object too that is in the HashFunction documentation. So it's smelling a little funky and overkill here to me. There is an alternative. The Digest only really matters from a testing perspective. I don't think it matters in a production environment. It's not like someone is storing the plan anywhere to be used later on. So if the tests were smart enough to ignore the hash part in the name, it would work too. I suppose the downside is that it could match any hash. But I'm not sure that is enough of a testing downside to matter either. The question is more how hard it is to code the testing environment to understand this. So I'm not sure. Lemme know what you think. -- To view, visit http://gerrit.cloudera.org:8080/24577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264 Gerrit-Change-Number: 24577 Gerrit-PatchSet: 15 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 17 Aug 2026 22:57:28 +0000 Gerrit-HasComments: Yes
