[ https://issues.apache.org/jira/browse/HIVE-27234?focusedWorklogId=859800&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-859800 ]
ASF GitHub Bot logged work on HIVE-27234: ----------------------------------------- Author: ASF GitHub Bot Created on: 29/Apr/23 17:29 Start Date: 29/Apr/23 17:29 Worklog Time Spent: 10m Work Description: zhangbutao commented on code in PR #4216: URL: https://github.com/apache/hive/pull/4216#discussion_r1181105965 ########## iceberg/iceberg-handler/src/test/queries/negative/alter_table_create_branch_negative.q: ########## @@ -0,0 +1,3 @@ +create table ice_tbl (id int, name string) Stored by Iceberg; + +alter table ice_tbl create branch test_branch_1; Review Comment: Qtest: creating branch on table without current snapshot will fail ########## iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergBranchOperation.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.mr.hive; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; +import org.apache.iceberg.SnapshotRef; +import org.apache.iceberg.Table; +import org.junit.Assert; +import org.junit.Test; + +public class TestHiveIcebergBranchOperation extends HiveIcebergStorageHandlerWithEngineBase { + + @Test + public void testCreateBranchWithDefaultConfig() throws InterruptedException, IOException { + Table table = + testTables.createTableWithVersions(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, + fileFormat, HiveIcebergStorageHandlerTestUtils.CUSTOMER_RECORDS, 2); + + String branchName = "test_branch_1"; + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s", branchName)); + table.refresh(); + SnapshotRef ref = table.refs().get(branchName); + Assert.assertEquals(table.currentSnapshot().snapshotId(), ref.snapshotId()); + Assert.assertNull(ref.minSnapshotsToKeep()); + Assert.assertNull(ref.maxSnapshotAgeMs()); + Assert.assertNull(ref.maxRefAgeMs()); + + // creating a branch which is already exists will fail + try { + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s", branchName)); Review Comment: Test: creating a branch which is already exists will fail ########## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ########## @@ -676,6 +678,35 @@ public void executeOperation(org.apache.hadoop.hive.ql.metadata.Table hmsTable, } } + @Override + public void createBranchOperation(org.apache.hadoop.hive.ql.metadata.Table hmsTable, + AlterTableCreateBranchSpec createBranchSpec) { + TableDesc tableDesc = Utilities.getTableDesc(hmsTable); + Table icebergTable = IcebergTableUtil.getTable(conf, tableDesc.getProperties()); + + String branchName = createBranchSpec.getBranchName(); + Optional.ofNullable(icebergTable.currentSnapshot()).orElseThrow(() -> new UnsupportedOperationException( Review Comment: Here give a check if table have current snapshot. ########## iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergBranchOperation.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.mr.hive; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; +import org.apache.iceberg.SnapshotRef; +import org.apache.iceberg.Table; +import org.junit.Assert; +import org.junit.Test; + +public class TestHiveIcebergBranchOperation extends HiveIcebergStorageHandlerWithEngineBase { + + @Test + public void testCreateBranchWithDefaultConfig() throws InterruptedException, IOException { + Table table = + testTables.createTableWithVersions(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, + fileFormat, HiveIcebergStorageHandlerTestUtils.CUSTOMER_RECORDS, 2); + + String branchName = "test_branch_1"; + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s", branchName)); + table.refresh(); + SnapshotRef ref = table.refs().get(branchName); + Assert.assertEquals(table.currentSnapshot().snapshotId(), ref.snapshotId()); + Assert.assertNull(ref.minSnapshotsToKeep()); + Assert.assertNull(ref.maxSnapshotAgeMs()); + Assert.assertNull(ref.maxRefAgeMs()); + + // creating a branch which is already exists will fail + try { + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s", branchName)); + } catch (Throwable e) { + while (e.getCause() != null) { + e = e.getCause(); + } + Assert.assertTrue(e.getMessage().contains("Ref test_branch_1 already exists")); + } + } + + @Test + public void testCreateBranchWithSnapshotId() throws InterruptedException, IOException { + Table table = + testTables.createTableWithVersions(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, + fileFormat, HiveIcebergStorageHandlerTestUtils.CUSTOMER_RECORDS, 2); + + String branchName = "test_branch_1"; + Long snapshotId = table.history().get(0).snapshotId(); + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s AS OF VERSION %d", + branchName, snapshotId)); + table.refresh(); + SnapshotRef ref = table.refs().get(branchName); + Assert.assertEquals(snapshotId.longValue(), ref.snapshotId()); + Assert.assertNull(ref.minSnapshotsToKeep()); + Assert.assertNull(ref.maxSnapshotAgeMs()); + Assert.assertNull(ref.maxRefAgeMs()); + } + + @Test + public void testCreateBranchWithMaxRefAge() throws InterruptedException, IOException { + Table table = + testTables.createTableWithVersions(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, + fileFormat, HiveIcebergStorageHandlerTestUtils.CUSTOMER_RECORDS, 2); + + String branchName = "test_branch_1"; + long maxRefAge = 5L; + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s RETAIN %d DAYS", + branchName, maxRefAge)); + table.refresh(); + SnapshotRef ref = table.refs().get(branchName); + Assert.assertEquals(table.currentSnapshot().snapshotId(), ref.snapshotId()); + Assert.assertNull(ref.minSnapshotsToKeep()); + Assert.assertNull(ref.maxSnapshotAgeMs()); + Assert.assertEquals(TimeUnit.DAYS.toMillis(maxRefAge), ref.maxRefAgeMs().longValue()); + } + + @Test + public void testCreateBranchWithMinSnapshotsToKeep() throws InterruptedException, IOException { + Table table = + testTables.createTableWithVersions(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, + fileFormat, HiveIcebergStorageHandlerTestUtils.CUSTOMER_RECORDS, 2); + + String branchName = "test_branch_1"; + Integer minSnapshotsToKeep = 2; + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s WITH SNAPSHOT RETENTION %d SNAPSHOTS", + branchName, minSnapshotsToKeep)); + table.refresh(); + SnapshotRef ref = table.refs().get(branchName); + Assert.assertEquals(table.currentSnapshot().snapshotId(), ref.snapshotId()); + Assert.assertEquals(minSnapshotsToKeep, ref.minSnapshotsToKeep()); + Assert.assertNull(ref.maxSnapshotAgeMs()); + Assert.assertNull(ref.maxRefAgeMs()); + } + + @Test + public void testCreateBranchWithMinSnapshotsToKeepAndMaxSnapshotAge() throws InterruptedException, IOException { + Table table = + testTables.createTableWithVersions(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, + fileFormat, HiveIcebergStorageHandlerTestUtils.CUSTOMER_RECORDS, 2); + + String branchName = "test_branch_1"; + Integer minSnapshotsToKeep = 2; + long maxSnapshotAge = 2L; + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s WITH SNAPSHOT RETENTION %d SNAPSHOTS" + + " %d DAYS", branchName, minSnapshotsToKeep, maxSnapshotAge)); + table.refresh(); + SnapshotRef ref = table.refs().get(branchName); + Assert.assertEquals(table.currentSnapshot().snapshotId(), ref.snapshotId()); + Assert.assertEquals(minSnapshotsToKeep, ref.minSnapshotsToKeep()); + Assert.assertEquals(TimeUnit.DAYS.toMillis(maxSnapshotAge), ref.maxSnapshotAgeMs().longValue()); + Assert.assertNull(ref.maxRefAgeMs()); + } + + @Test + public void testCreateBranchWithAllCustomConfig() throws InterruptedException, IOException { + Table table = + testTables.createTableWithVersions(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, + fileFormat, HiveIcebergStorageHandlerTestUtils.CUSTOMER_RECORDS, 2); + + String branchName = "test_branch_1"; + Long snapshotId = table.history().get(0).snapshotId(); + Integer minSnapshotsToKeep = 2; + long maxSnapshotAge = 2L; + long maxRefAge = 5L; + shell.executeStatement(String.format("ALTER TABLE customers CREATE BRANCH %s AS OF VERSION %d RETAIN %d DAYS WITH" + + " SNAPSHOT RETENTION %d SNAPSHOTS %d days", + branchName, snapshotId, maxRefAge, minSnapshotsToKeep, maxSnapshotAge)); + table.refresh(); + SnapshotRef ref = table.refs().get(branchName); + Assert.assertEquals(snapshotId.longValue(), ref.snapshotId()); + Assert.assertEquals(minSnapshotsToKeep, ref.minSnapshotsToKeep()); + Assert.assertEquals(TimeUnit.DAYS.toMillis(maxSnapshotAge), ref.maxSnapshotAgeMs().longValue()); + Assert.assertEquals(TimeUnit.DAYS.toMillis(maxRefAge), ref.maxRefAgeMs().longValue()); + } + + @Test + public void testCreateBranchWithNonIcebergTable() { + shell.executeStatement("create table nonice_tbl (id int, name string)"); + + String branchName = "test_branch_1"; + try { + shell.executeStatement(String.format("ALTER TABLE nonice_tbl CREATE BRANCH %s", branchName)); Review Comment: Test: creating branch on a non-icebeg table will fail Issue Time Tracking ------------------- Worklog Id: (was: 859800) Time Spent: 4h 20m (was: 4h 10m) > Iceberg: CREATE BRANCH SQL implementation > ------------------------------------------ > > Key: HIVE-27234 > URL: https://issues.apache.org/jira/browse/HIVE-27234 > Project: Hive > Issue Type: Sub-task > Components: Iceberg integration > Reporter: zhangbutao > Assignee: zhangbutao > Priority: Major > Labels: pull-request-available > Time Spent: 4h 20m > Remaining Estimate: 0h > > Maybe we can follow spark sql about branch ddl implementation > [https://github.com/apache/iceberg/pull/6617] -- This message was sent by Atlassian Jira (v8.20.10#820010)