[Iceberg]Support view on Rest catalog and Nessie catalog

[hantangwangd]

Description

Iceberg's RestCatalog and NessieCatalog have implemented ViewCatalog interface, that means we can support operations on view for Iceberg connector configured with these catalogs. This PR refactor IcebergNativeMetadata, so that catalogs like REST and NESSIE that implements interface ViewCatalog can now support operations on view.

Motivation and Context

Support view for Iceberg connector on as many catalog types as possible

Impact

Iceberg connector configured with REST and NESSIE catalogs can now support operations on view

Test Plan

• Enable existing test cases for views on REST and NESSIE

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

== RELEASE NOTES ==

Iceberg Connector Changes
* Add support of ``view`` for Iceberg connector when configured with ``REST`` and ``NESSIE``. :pr:`23793`

[tdcmeehan]

Instead of catching this exception, does it make sense to add a new method, isIcebergView, then just load the view directly if it's true?

[hantangwangd]

Yes, it seems more natural this way. But I did not find an efficient way to implement method isIcebergView. ViewCatalog in Iceberg library provide a method boolean viewExists(TableIdentifier identifier), based on this method, we can implement isIcebergView as follows:

boolean isIcebergView(......, SchemaTableName table) {
    ......

    if (catalog instanceof ViewCatalog) {
        return ((ViewCatalog) catalog).viewExists(toIcebergTableIdentifier(table));
    }

    return false;
}

However, when we examine ViewCatalog.viewExists(identifier) provided by Iceberg library, we find that its implementation is based on loadView(idenfifier) as well:

boolean viewExists(TableIdentifier identifier) {
    try {
      loadView(identifier);
      return true;
    } catch (NoSuchViewException e) {
      return false;
    }
  }

Considering this situation, the above way doesn't seem very efficient, it will do a lot of additional view loading, whether we are querying tables or views.

So the way of capturing exceptions may not seem so natural, but it's an efficient way as far as I can think of. What do you think, which one is more preferable? And open for any other thoughts.

[tdcmeehan]

Agreed, thanks for the explanation.

[tdcmeehan]

Can you add some of this information as a comment so folks who read this code later will understand this?

[hantangwangd]

Sure, the comment has been added. Please take a look when available, thanks.

[steveburnett]

Thanks for the release note entry! Nit rephrase suggestion to follow the Order of changes in the Release Notes Guideline.

== RELEASE NOTES ==

Iceberg Connector Changes
* Add support of `view` for Iceberg connector when configured with `REST` and `NESSIE`. :pr:`23793`

[tdcmeehan]

Couple nits, otherwise LGTM

[hantangwangd]

@steveburnett Thanks for the suggestion, fixed! Please take a look when available.

[steveburnett]

@steveburnett Thanks for the suggestion, fixed! Please take a look when available.

LGTM, thanks!

[tdcmeehan]

Some other very small nits, LGTM

[kiersten-stokes]

This looks great! I know this is a sought-after feature in the community. My only suggestion would be to add a blurb to the Iceberg connector documentation, since right now it only indicates view availability in Hive/Glue

[hantangwangd]

@kiersten-stokes Thanks for your suggestion, I have supplemented the Iceberg document, please take a look when available.

[hantangwangd]

Some other very small nits, LGTM

Comments has been fixed, please take a look when available, thanks! @tdcmeehan

[steveburnett]

LGTM! (docs)

Pull branch, review local doc build. Thanks!

[kiersten-stokes]

LGTM! Thank you

[steveburnett]

Noticed a few nits in formatting for the release note entry - tiny, but fixing it now saves time when the next release note PR is done.

== RELEASE NOTES ==

Iceberg Connector Changes
* Add support of ``view`` for Iceberg connector when configured with ``REST`` and ``NESSIE``. :pr:`23793`

[hantangwangd]

Noticed a few nits in formatting for the release note entry - tiny, but fixing it now saves time when the next release note PR is done.

== RELEASE NOTES ==

Iceberg Connector Changes
* Add support of ``view`` for Iceberg connector when configured with ``REST`` and ``NESSIE``. :pr:`23793`

Fixed, thanks for your suggestion. @steveburnett

[ZacBlanco]

two minor things

[hantangwangd]

Hi Zac @ZacBlanco, after recheck the code, I confirm that an iceberg Table or View instance could be a big object in memory, as it has a dedicated TableOperations instance which holds the current TableMetadata parsed from table metadata file, this could be somewhat large. However, ConnectorMetadata is an object whose lifespan is within the scope of a transaction, which means IcebergAbstractMetadata instances are not global. So it seems that we needn't realize a cache with max entries count limit for tables and views in IcebergAbstractMetadata, since only the ones that are used in current transaction would be maintained, the number is unlikely to be very much. What do you think, is it reasonable?

[ZacBlanco]

I think it is reasonable to not have a cache then