pvary commented on code in PR #17317:
URL: https://github.com/apache/iceberg/pull/17317#discussion_r3639403033
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -1266,4 +1271,108 @@ public void testMetadataHashing(boolean
isTableEncrypted) {
assertThat(base64EncodedHash).isNull();
}
}
+
+ // The tests below exercise the Iceberg-only listing path
(listIcebergTablesByFilter). Hive
+ // table/listing filtering is off by default and relies on the HMS-side
+ // `get_table_names_by_filter`
+ // filter, so we assert that non-Iceberg objects are excluded from
listTables even
+ // when other tables/views co-exist in the same database.
+
+ @Test
+ public void testListTablesFiltersOutNonIcebergTables() throws TException,
IOException {
+ Namespace ns = Namespace.of(DB_NAME);
+ TableIdentifier icebergTable = TableIdentifier.of(ns, "iceberg_tbl");
+ TableIdentifier hiveTable = TableIdentifier.of(ns, "hive_tbl");
+
+ catalog.createTable(icebergTable, getTestSchema());
+ HIVE_METASTORE_EXTENSION
+ .metastoreClient()
+ .createTable(createHiveTable(hiveTable.name(),
TableType.EXTERNAL_TABLE));
+
+ try {
+ assertThat(catalog.listTables(ns)).containsExactly(icebergTable);
+ } finally {
+ catalog.dropTable(icebergTable);
+ HIVE_METASTORE_EXTENSION.metastoreClient().dropTable(DB_NAME,
hiveTable.name());
+ }
+ }
+
+ @Test
+ public void testListTablesByFilterReturnsAllIcebergTables() throws Exception
{
+ Namespace ns = Namespace.of(DB_NAME);
+ TableIdentifier t1 = TableIdentifier.of(ns, "t1");
+ TableIdentifier t2 = TableIdentifier.of(ns, "t2");
+
+ catalog.createTable(t1, getTestSchema());
+ catalog.createTable(t2, getTestSchema());
+
+ try {
+ assertThat(catalog.listTables(ns)).containsExactlyInAnyOrder(t1, t2);
+ } finally {
+ catalog.dropTable(t1);
+ catalog.dropTable(t2);
+ }
+ }
+
+ @Test
+ public void testListTablesFiltersNonIcebergTables() throws TException,
IOException {
+ // The server-side filter should exclude a non-Iceberg table that
co-exists with an Iceberg
+ // table in the same database.
+ Namespace ns = Namespace.of(DB_NAME);
+ TableIdentifier icebergTable = TableIdentifier.of(ns, "iceberg_tbl");
+ TableIdentifier hiveTable = TableIdentifier.of(ns, "hive_tbl");
+
+ catalog.createTable(icebergTable, getTestSchema());
+ HIVE_METASTORE_EXTENSION
+ .metastoreClient()
+ .createTable(createHiveTable(hiveTable.name(),
TableType.EXTERNAL_TABLE));
+
+ try {
+ assertThat(catalog.listTables(ns))
+ .as("server-side filter should exclude the hive table")
+ .containsExactly(icebergTable);
+ } finally {
+ catalog.dropTable(icebergTable);
+ HIVE_METASTORE_EXTENSION.metastoreClient().dropTable(DB_NAME,
hiveTable.name());
+ }
+ }
+
+ private org.apache.hadoop.hive.metastore.api.Table createHiveTable(
+ String hiveTableName, TableType type) throws IOException {
+ Map<String, String> parameters = Maps.newHashMap();
+ parameters.put(
+ serdeConstants.SERIALIZATION_CLASS,
"org.apache.hadoop.hive.serde2.thrift.test.IntString");
+ parameters.put(
+ serdeConstants.SERIALIZATION_FORMAT,
"org.apache.thrift.protocol.TBinaryProtocol");
+
+ SerDeInfo serDeInfo =
+ new SerDeInfo(null,
"org.apache.hadoop.hive.serde2.thrift.ThriftDeserializer", parameters);
Review Comment:
Do we need all of these parameters?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
