Skip to content

Remove ref hash session property for Iceberg Nessie catalog#21501

Closed
agrawalreetika wants to merge 1 commit intoprestodb:masterfrom
agrawalreetika:nessie-ref-hash
Closed

Remove ref hash session property for Iceberg Nessie catalog#21501
agrawalreetika wants to merge 1 commit intoprestodb:masterfrom
agrawalreetika:nessie-ref-hash

Conversation

@agrawalreetika
Copy link
Member

Description

Remove ref hash session property for Iceberg Nessie catalog

Motivation and Context

This came from the discussion in #21399 (comment)
We can discuss if we want to remove ref hash as session property from Presto or we want to modify existing support.

Impact

None

Test Plan

NA

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== NO RELEASE NOTE ==

@agrawalreetika
Copy link
Member Author

@nastra @ajantha-bhat I was not able to locate in reviewers. Please check this.

@yingsu00 yingsu00 added the iceberg Apache Iceberg related label Dec 13, 2023
Copy link
Contributor

@yingsu00 yingsu00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agrawalreetika I checked Nessie documentation and source code, and it seems the ref hash is something like the commit hash on a git branch. If there are 2 new commits on this branch, lets say branch A, then the reference to the first commit would be (name="A", hash=[hash_commit_1]), and the reference to the latest commit is (name="A", hash=[hash_commit_2]) or just (name="A"). The "Hash on nessie.ref" in https://projectnessie.org/tools/client_config/ is not "Hash OF the nessie.ref name", but the hash of one of the commits on the reference branch. I mistakenly thought it was "Hash OF the nessie.ref name" before.

If my current understanding is correct, then we should not remove the ref hash property, but instead, we shall make it settable and validate the user's input. Nessie lib provides hash validation API, check Validation.validateHash() and validateHashOrRelativeSpec() methods. Though I think the parameter name "referenceName" in Validation.validateHash(String referenceName) is confusing. Maybe we can send a PR to Nessie to change it to "hash" instead.

cc @tdcmeehan

@agrawalreetika
Copy link
Member Author

@yingsu00 Thanks for the details. Make sense, I will explore around the validation API of Nessie. In that case, we can close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

iceberg Apache Iceberg related

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants