Hive: Refactor HiveTableOperations with common code for View.

[nk1506]

As part of issues #8698 , as discussed here , All the common piece between table and view has been moved from HiveTableOperations to a new helper class.

[pvary]

How will this work for View-s?
Any thoughts here?

[nk1506]

In case of Table envContext is looking like here .
expectedMetadataLocation != null ? ImmutableMap.of( NO_LOCK_EXPECTED_KEY, METADATA_LOCATION_PROP, NO_LOCK_EXPECTED_VALUE, expectedMetadataLocation) : ImmutableMap.of()

In case of View it will look like
expectedMetadataLocation != null ? ImmutableMap.of( BaseMetastoreTableOperations.METADATA_LOCATION_PROP, expectedMetadataLocation) : ImmutableMap.of()

[pvary]

The goal of this setting is to send a map to Hive, that only execute the update, if the METADATA_LOCATION_PROP is still expectedMetadataLocation.

If for the views we still use the metadata location, then we probably want to use the same config here as for the tables.

[pvary]

I have one question left, which is somewhat related to the new usage of the HiveOperationsBase

[pvary]

I am really sorry for missing this before.

Should we set this generally in the newHmsTable?

[nk1506]

Yes, it was fetching tableOwner from TableMetadata.

[pvary]

Don't we have the same tableOwner for views as well?

[nk1506]

For views, I thought it's always going to be CurrentUser. But I think it makes sense to get the tableOwner from ViewMetadata as well if Present.
Still we will have to expect user information from Caller of newHmsTable , right?

[pvary]

You might be better positioned to suggest things from the View point of view 😄, but if it were only me, then I would have created the following method in HiveOperationsBase:

default Table newHmsTable(TableMetadata metadata) {
[..]
}

and kept the implementation the same (using the same metadata key to get the owner), like before:

    Preconditions.checkNotNull(metadata, "'metadata' parameter can't be null");
    final long currentTimeMillis = System.currentTimeMillis();

    Table newTable =
        new Table(
            tableName,
            database,
            metadata.property(HiveCatalog.HMS_TABLE_OWNER, HiveHadoopUtil.currentUser()),
            (int) currentTimeMillis / 1000,
            (int) currentTimeMillis / 1000,
            Integer.MAX_VALUE,
            null,
            Collections.emptyList(),
            Maps.newHashMap(),
            null,
            null,
            TableType.EXTERNAL_TABLE.toString());

    newTable
        .getParameters()
        .put("EXTERNAL", "TRUE"); // using the external table type also requires this
    return newTable;

[nk1506]

TableMetadata and ViewMetadata both are unrelated. I am not able to find a better approach with newHmsTable(TableMetadata metadata) .
Please suggest.

[pvary]

Probably it would be the best to have a common interface for the 2, but we should not hold back this part for it.

[pvary]

Thanks for all the work and patience @nk1506 !

[nk1506]

Thanks @pvary for suggesting and reviewing this. I will start rebasing View PR.