Spark: Update register table procedure test case for all the catalogs #6787
Conversation
| } | ||
|
|
||
| @Test | ||
| public void testRegisterTable() throws NoSuchTableException, ParseException { |
There was a problem hiding this comment.
So we aren't actually adding any more tests here we are just removing this Assume clause?
There was a problem hiding this comment.
Yes.
Already API level test is there for each catalog for register table.
Configuring other catalogs in this test case is hard (like Glue, REST). It is better to keep them as integration tests (may in in a follow up PR)
Now removing this check will make sure that this testcase runs for hadoop, hive, spark catalog.
There was a problem hiding this comment.
How do we Register a table in Hadoop? That doesn't sound possible
There was a problem hiding this comment.
HadoopCatalog also extends BaseMetastoreCatalog
So, when a register table is called. It makes a fresh commit to the Hadoop catalog too (file name will be as per hadoop catalog)
https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java#L84
There was a problem hiding this comment.
Existing testcase for hadoop catalog
https://github.com/apache/iceberg/blob/master/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java#L637
There was a problem hiding this comment.
OK Got it, Still not sure I think that was a good idea but it's already in.
2 participants
PR #5037 supports register tables for all (most of) the catalogs.
There is no restriction in the register table procedure about catalog type. But the testcase has a restriction.
Hence, update the testcase to test for all the catalogs.