You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/08/14 18:16:05 UTC
[GitHub] [iceberg] RussellSpitzer opened a new pull request #1342: Static Table Operations
RussellSpitzer opened a new pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342
Allows for a Table Operations which references a specfic metadata version
file. This operation will not change even if the base table it was derived
from is changed. This enables it to act like a ReadOnly view of the table's
state at a given time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#issuecomment-676579631
Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470832106
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
Review comment:
Sounds good to me, I had a lot of difficulty with verbiage in comments for this
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470835468
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -68,27 +71,26 @@ public HadoopTables(Configuration conf) {
*/
@Override
public Table load(String location) {
- TableOperations ops = newTableOps(location);
- if (ops.current() == null) {
+ //Possibly Load a Metadata Table
Review comment:
How about parse out the metadata table name, if present?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470834638
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
Review comment:
I'm not sure we need both this and the next test, but it's okay to have them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470863641
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
Review comment:
Got it!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470848280
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
+ }
+
+ @Test
+ public void testMetadataTables() {
+ for (MetadataTableType type: MetadataTableType.values()) {
+ String enumName = type.name().replace("_","").toLowerCase();
Review comment:
we are just comparing the ClassName here, so we are comparing AllDataFilesTable.class with ALL_DATA_FILES
I may be misunderstanding your comment here, I'm just making sure that we get back the Class with the right name based on the MetadataType we request.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470840676
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
+ */
+public class StaticTableOperations implements TableOperations {
+ private final TableMetadata staticMetadata;
+ private final FileIO io;
+
+ /**
+ * Creates a StaticTableOperations tied to a specific static version of the TableMetadata
+ */
+ public StaticTableOperations(String location, FileIO io) {
+ this.io = io;
+ this.staticMetadata = TableMetadataParser.read(io, location);
+ }
+
+ @Override
+ public TableMetadata current() {
+ return staticMetadata;
+ }
+
+ @Override
+ public TableMetadata refresh() {
+ return staticMetadata;
+ }
+
+ @Override
+ public void commit(TableMetadata base, TableMetadata metadata) {
+ throw new UnsupportedOperationException("This TableOperations is static, it cannot be modified");
Review comment:
sounds good to me
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470834476
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
Review comment:
We typically use `AssertHelpers.assertThrows` instead of `expected` because that allows you to validate the exception message and make assertions after the failure. For example, this should assert that the value of `table.current()` before the commit is the same object as `table.refresh()` after the failed commit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470836071
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
+ */
+public class StaticTableOperations implements TableOperations {
+ private final TableMetadata staticMetadata;
+ private final FileIO io;
+
+ /**
+ * Creates a StaticTableOperations tied to a specific static version of the TableMetadata
+ */
+ public StaticTableOperations(String location, FileIO io) {
+ this.io = io;
+ this.staticMetadata = TableMetadataParser.read(io, location);
+ }
+
+ @Override
+ public TableMetadata current() {
+ return staticMetadata;
+ }
+
+ @Override
+ public TableMetadata refresh() {
+ return staticMetadata;
+ }
+
+ @Override
+ public void commit(TableMetadata base, TableMetadata metadata) {
+ throw new UnsupportedOperationException("This TableOperations is static, it cannot be modified");
Review comment:
Users don't know about `TableOperations`, so we shouldn't mention it here. How about `Cannot modify a static table`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470835743
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -68,27 +71,26 @@ public HadoopTables(Configuration conf) {
*/
@Override
public Table load(String location) {
- TableOperations ops = newTableOps(location);
Review comment:
I like this. It is more straightforward than the previous implementation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#issuecomment-676568076
Looks great! Thank you @RussellSpitzer!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#issuecomment-676532109
@rdblue just a friendly ping, once this gets in I"ll redo the ExpireSnapshots branch, that should be very straightforward
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#issuecomment-674246894
Thanks @RussellSpitzer! Overall this is the right approach and I think the implementation looks good. Just a few minor things.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470864810
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
+ */
+public class StaticTableOperations implements TableOperations {
+ private final TableMetadata staticMetadata;
+ private final FileIO io;
+
+ /**
+ * Creates a StaticTableOperations tied to a specific static version of the TableMetadata
+ */
+ public StaticTableOperations(String location, FileIO io) {
+ this.io = io;
+ this.staticMetadata = TableMetadataParser.read(io, location);
+ }
+
+ @Override
+ public TableMetadata current() {
+ return staticMetadata;
+ }
+
+ @Override
+ public TableMetadata refresh() {
+ return staticMetadata;
+ }
+
+ @Override
+ public void commit(TableMetadata base, TableMetadata metadata) {
+ throw new UnsupportedOperationException("This TableOperations is static, it cannot be modified");
+ }
+
+ @Override
+ public FileIO io() {
+ return this.io;
+ }
+
+ @Override
+ public String metadataFileLocation(String fileName) {
+ throw new UnsupportedOperationException("New files cannot be created in a Static Table Operations");
Review comment:
Gonna just switch them all to "cannot modify" since this would come up even when deleting files.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470836678
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
+ */
+public class StaticTableOperations implements TableOperations {
+ private final TableMetadata staticMetadata;
+ private final FileIO io;
+
+ /**
+ * Creates a StaticTableOperations tied to a specific static version of the TableMetadata
+ */
+ public StaticTableOperations(String location, FileIO io) {
+ this.io = io;
+ this.staticMetadata = TableMetadataParser.read(io, location);
+ }
+
+ @Override
+ public TableMetadata current() {
+ return staticMetadata;
+ }
+
+ @Override
+ public TableMetadata refresh() {
+ return staticMetadata;
+ }
+
+ @Override
+ public void commit(TableMetadata base, TableMetadata metadata) {
+ throw new UnsupportedOperationException("This TableOperations is static, it cannot be modified");
+ }
+
+ @Override
+ public FileIO io() {
+ return this.io;
+ }
+
+ @Override
+ public String metadataFileLocation(String fileName) {
+ throw new UnsupportedOperationException("New files cannot be created in a Static Table Operations");
Review comment:
Same here, we could clean these messages up a bit, "Cannot create new files for a static table".
The actual message is fine, but we try to structure them as `"Cannot" + (what you tried) + ": " reason and context`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470833358
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
Review comment:
This would be true of any different table instance that refers to the same table because `refresh` was not called. I think this test should call `staticTable.refresh()` after the modifications to test what you intend.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470837315
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
+ }
+
+ @Test
+ public void testMetadataTables() {
+ for (MetadataTableType type: MetadataTableType.values()) {
+ String enumName = type.name().replace("_","").toLowerCase();
Review comment:
I thought those were loaded using `all_data_files`, `all_manifests`, etc. and not `alldatafiles`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470833716
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
+ }
+
+ @Test
+ public void testMetadataTables() {
+ for (MetadataTableType type: MetadataTableType.values()) {
+ String enumName = type.name().replace("_","").toLowerCase();
Review comment:
When is this needed?
Also, style nit: no space between arguments to `replace`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470853589
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -68,27 +71,26 @@ public HadoopTables(Configuration conf) {
*/
@Override
public Table load(String location) {
- TableOperations ops = newTableOps(location);
- if (ops.current() == null) {
+ //Possibly Load a Metadata Table
+ if (location.contains("#") && !location.endsWith("#")) {
// try to resolve a metadata table, which we encode as URI fragments
// e.g. hdfs:///warehouse/my_table#snapshots
int hashIndex = location.lastIndexOf('#');
- if (hashIndex != -1 && location.length() - 1 != hashIndex) {
- // we found char '#', and it is not the last char of location
- String baseTable = location.substring(0, hashIndex);
- String metaTable = location.substring(hashIndex + 1);
- MetadataTableType type = MetadataTableType.from(metaTable);
- if (type != null) {
- return loadMetadataTable(baseTable, type);
- } else {
- throw new NoSuchTableException("Table does not exist at location: " + location);
- }
- } else {
- throw new NoSuchTableException("Table does not exist at location: " + location);
+ String baseTable = location.substring(0, hashIndex);
+ String metaTable = location.substring(hashIndex + 1);
+ MetadataTableType type = MetadataTableType.from(metaTable);
+ if (type != null) {
+ return loadMetadataTable(baseTable, type);
}
}
- return new BaseTable(ops, location);
+ //Normal Table Load if we haven't loaded a MetadataTable
Review comment:
cleaned up!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470834157
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
+ }
+
+ @Test
+ public void testMetadataTables() {
+ for (MetadataTableType type: MetadataTableType.values()) {
+ String enumName = type.name().replace("_","").toLowerCase();
Review comment:
ALL_DATA_FILES, will fix style
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470857032
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -68,27 +71,26 @@ public HadoopTables(Configuration conf) {
*/
@Override
public Table load(String location) {
- TableOperations ops = newTableOps(location);
- if (ops.current() == null) {
+ //Possibly Load a Metadata Table
Review comment:
I made a different implementation here where we will instead do
```
Name, Type = tryParseMetadataLocation
if (name, type).{
loadMetadata
} else {
loadBasic
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470857411
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
Review comment:
Oh I see there is a custom Iceberg version of it! I get things
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470834157
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
+ }
+
+ @Test
+ public void testMetadataTables() {
+ for (MetadataTableType type: MetadataTableType.values()) {
+ String enumName = type.name().replace("_","").toLowerCase();
Review comment:
ALL_DATA_FILES, ALL_MANIFESTS, ALL_ENTRIES
All the style things just make me want to upgrade the Checkstyle version even more :) I forgot to run in Intellij where I can force the newer version
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470832379
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -68,27 +71,26 @@ public HadoopTables(Configuration conf) {
*/
@Override
public Table load(String location) {
- TableOperations ops = newTableOps(location);
- if (ops.current() == null) {
+ //Possibly Load a Metadata Table
+ if (location.contains("#") && !location.endsWith("#")) {
// try to resolve a metadata table, which we encode as URI fragments
// e.g. hdfs:///warehouse/my_table#snapshots
int hashIndex = location.lastIndexOf('#');
- if (hashIndex != -1 && location.length() - 1 != hashIndex) {
- // we found char '#', and it is not the last char of location
- String baseTable = location.substring(0, hashIndex);
- String metaTable = location.substring(hashIndex + 1);
- MetadataTableType type = MetadataTableType.from(metaTable);
- if (type != null) {
- return loadMetadataTable(baseTable, type);
- } else {
- throw new NoSuchTableException("Table does not exist at location: " + location);
- }
- } else {
- throw new NoSuchTableException("Table does not exist at location: " + location);
+ String baseTable = location.substring(0, hashIndex);
+ String metaTable = location.substring(hashIndex + 1);
+ MetadataTableType type = MetadataTableType.from(metaTable);
+ if (type != null) {
+ return loadMetadataTable(baseTable, type);
}
}
- return new BaseTable(ops, location);
+ //Normal Table Load if we haven't loaded a MetadataTable
Review comment:
A couple of style nits: There should be a space before the start of the comment text, and we typically use sentence case, unless referring to specific objects, like MetadataTable (no need to capitalize "Table" and "Load").
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470788727
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -68,27 +71,26 @@ public HadoopTables(Configuration conf) {
*/
@Override
public Table load(String location) {
- TableOperations ops = newTableOps(location);
Review comment:
An alternative to this implementation that I considered was modifying findTable itself. The problem with this is that to do that implementation we need to extract the loadMetadataTable from below. I think this is a cleaner solution but we could do something where we make a StaticTables.java which findTables uses to either return a basetable or metadataTable
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#issuecomment-674289041
Updated
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470845743
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
Review comment:
Ah weird, for some reason IntelliJ told me I only had Junit4 on the class path so I didn't use Assert.assertThrows. That's why I only am using Junit4 conventions :/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470836450
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
+ */
+public class StaticTableOperations implements TableOperations {
+ private final TableMetadata staticMetadata;
+ private final FileIO io;
+
+ /**
+ * Creates a StaticTableOperations tied to a specific static version of the TableMetadata
+ */
+ public StaticTableOperations(String location, FileIO io) {
Review comment:
metadataFileLocation? That would mirror the TableMetadata object
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470831036
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
Review comment:
I'm not sure that I would say that the `TableOperations` implementation is a representation, and it is probably a good idea to avoid using "snapshot" to mean something other than a `Snapshot`. How about "`TableOperations ` implementation that provides access to metadata for a `Table` at some point in time, using a table metadata location"?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470859700
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
+ }
+
+ @Test
+ public void testMetadataTables() {
+ for (MetadataTableType type: MetadataTableType.values()) {
+ String enumName = type.name().replace("_","").toLowerCase();
Review comment:
Okay, I see. Nevermind, then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470864015
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -68,27 +71,26 @@ public HadoopTables(Configuration conf) {
*/
@Override
public Table load(String location) {
- TableOperations ops = newTableOps(location);
- if (ops.current() == null) {
+ //Possibly Load a Metadata Table
+ if (location.contains("#") && !location.endsWith("#")) {
// try to resolve a metadata table, which we encode as URI fragments
// e.g. hdfs:///warehouse/my_table#snapshots
int hashIndex = location.lastIndexOf('#');
- if (hashIndex != -1 && location.length() - 1 != hashIndex) {
- // we found char '#', and it is not the last char of location
- String baseTable = location.substring(0, hashIndex);
- String metaTable = location.substring(hashIndex + 1);
- MetadataTableType type = MetadataTableType.from(metaTable);
- if (type != null) {
- return loadMetadataTable(baseTable, type);
- } else {
- throw new NoSuchTableException("Table does not exist at location: " + location);
- }
- } else {
- throw new NoSuchTableException("Table does not exist at location: " + location);
+ String baseTable = location.substring(0, hashIndex);
+ String metaTable = location.substring(hashIndex + 1);
+ MetadataTableType type = MetadataTableType.from(metaTable);
+ if (type != null) {
+ return loadMetadataTable(baseTable, type);
}
}
- return new BaseTable(ops, location);
+ //Normal Table Load if we haven't loaded a MetadataTable
Review comment:
Cleaned it up
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470829747
##########
File path: core/src/main/java/org/apache/iceberg/StaticTableOperations.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.nio.file.Paths;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.LocationProvider;
+
+/**
+ * Representation of an immutable snapshot of Table State that can be used in
+ * other Iceberg Functions.
+ */
+public class StaticTableOperations implements TableOperations {
+ private final TableMetadata staticMetadata;
+ private final FileIO io;
+
+ /**
+ * Creates a StaticTableOperations tied to a specific static version of the TableMetadata
+ */
+ public StaticTableOperations(String location, FileIO io) {
Review comment:
Should this be `metadataLocation` to be more specific?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#discussion_r470854445
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestStaticTable.java
##########
@@ -0,0 +1,79 @@
+package org.apache.iceberg.hadoop;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.MetadataTableType;
+import org.apache.iceberg.StaticTableOperations;
+import org.apache.iceberg.Table;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStaticTable extends HadoopTableTestBase {
+
+ private Table getStaticTable() {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation());
+ }
+
+ private Table getStaticTable(MetadataTableType type) {
+ return TABLES.load(((HasTableOperations) table).operations().current().metadataFileLocation() + "#" + type);
+ }
+
+ @Test
+ public void testLoadFromMetadata() {
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Loading a metadata file based table should return StaticTableOperations",
+ ((HasTableOperations) staticTable).operations() instanceof StaticTableOperations);
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeAddedTo(){
+ Table staticTable = getStaticTable();
+ staticTable.newOverwrite().addFile(FILE_A).commit();
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testCannotBeDeletedFrom(){
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ staticTable.newDelete().deleteFile(FILE_A).commit();
+ }
+
+ @Test
+ public void testHasSameProperties(){
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+ Table staticTable = getStaticTable();
+ Assert.assertTrue("Same history?",
+ table.history().containsAll(staticTable.history()));
+ Assert.assertTrue("Same snapshot?",
+ table.currentSnapshot().snapshotId() == staticTable.currentSnapshot().snapshotId());
+ Assert.assertTrue("Same properties?",
+ Maps.difference(table.properties(), staticTable.properties()).areEqual());
+ }
+
+ @Test
+ public void testImmutable() {
+ table.newAppend().appendFile(FILE_A).commit();
+ Table staticTable = getStaticTable();
+ long originalSnapshot = table.currentSnapshot().snapshotId();
+
+ table.newAppend().appendFile(FILE_B).commit();
+ table.newOverwrite().deleteFile(FILE_B).addFile(FILE_C).commit();
+
+ Assert.assertEquals("Snapshot unchanged after table modified",
+ staticTable.currentSnapshot().snapshotId(), originalSnapshot);
Review comment:
Sgtm
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #1342: Static Table Operations
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1342:
URL: https://github.com/apache/iceberg/pull/1342#issuecomment-674203642
@rdblue + @aokolnychyi As we were talking about, a StaticTableOperations class for implementing #1344
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org