kevinjqliu commented on code in PR #1251:
URL: https://github.com/apache/iceberg-python/pull/1251#discussion_r1824793043
##########
pyiceberg/catalog/hive.py:
##########
@@ -407,7 +407,30 @@ def register_table(self, identifier: Union[str,
Identifier], metadata_location:
raise NotImplementedError
def list_views(self, namespace: Union[str, Identifier]) ->
List[Identifier]:
- raise NotImplementedError
+ """List Iceberg views under the given namespace in the catalog.
+
+ When the database doesn't exist, it will just return an empty list.
+
+ Args:
+ namespace: Database to list.
+
+ Returns:
+ List[Identifier]: List of view identifiers.
+
+ Raises:
+ NoSuchNamespaceError: If a namespace with the given name does not
exist, or the identifier is invalid.
+ """
+ database_name = self.identifier_to_database(namespace,
NoSuchNamespaceError)
+ with self._client as open_client:
+ return [
+ (database_name, view.tableName)
+ for view in open_client.get_table_objects_by_name(
+ dbname=database_name,
+ tbl_names=open_client.get_all_tables(db_name=database_name)
+ )
+ if view.parameters.get(TABLE_TYPE, "").lower() == ICEBERG and
view.tableType.lower() == "virtual_view"
Review Comment:
nit: can we do something like this here?
https://github.com/apache/iceberg-python/commit/d5b51f9eb14c6d8b8547b9bcdd3ec3059bdf3236#diff-5dc4045c0eac182d6c22d8a06f62fcc79c369ad8bff80d357252147e2d9a76f2R785
for both `__is_iceberg_table` and `__is_iceberg_view`
##########
tests/catalog/test_hive.py:
##########
@@ -1233,3 +1233,84 @@ def test_create_hive_client_failure() -> None:
with pytest.raises(Exception, match="Connection failed"):
HiveCatalog._create_hive_client(properties)
assert mock_hive_client.call_count == 2
+
+def test_list_views(hive_table: HiveTable) -> None:
+ catalog = HiveCatalog(HIVE_CATALOG_NAME, uri=HIVE_METASTORE_FAKE_URL)
+
+ view1 = deepcopy(hive_table)
+ view1.tableName = "view1"
+ view1.dbName = "database"
+ view1.tableType = "VIEW"
+ view1.parameters["table_type"] = "ICEBERG"
+
+ view2 = deepcopy(hive_table)
+ view2.tableName = "view2"
+ view2.dbName = "database"
+ view2.tableType = "VIEW"
+ view2.parameters["table_type"] = "ICEBERG"
+
+ non_iceberg_view = deepcopy(hive_table)
+ non_iceberg_view.tableName = "non_iceberg_view"
+ non_iceberg_view.dbName = "database"
+ non_iceberg_view.tableType = "VIEW"
+ non_iceberg_view.parameters["table_type"] = "non_iceberg"
+
+ non_view_table = deepcopy(hive_table)
+ non_view_table.tableName = "table"
+ non_view_table.dbName = "database"
+ non_view_table.tableType = "TABLE"
+ non_view_table.parameters["table_type"] = "ICEBERG"
+
+ catalog._client = MagicMock()
+ catalog._client.__enter__().get_all_tables.return_value = ["view1",
"view2", "non_iceberg_view", "table"]
+ catalog._client.__enter__().get_table_objects_by_name.return_value =
[view1, view2, non_iceberg_view, non_view_table]
+
+ got_views = catalog.list_views("database")
+ assert got_views == [("database", "view1"), ("database", "view2")]
+
+
catalog._client.__enter__().get_all_tables.assert_called_with(db_name="database")
+ catalog._client.__enter__().get_table_objects_by_name.assert_called_with(
+ dbname="database", tbl_names=["view1", "view2", "non_iceberg_view",
"table"]
+ )
+def test_list_views(hive_table: HiveTable)-> None:
Review Comment:
you can run `make lint` to fix linter issue
##########
tests/catalog/test_hive.py:
##########
@@ -1233,3 +1233,84 @@ def test_create_hive_client_failure() -> None:
with pytest.raises(Exception, match="Connection failed"):
HiveCatalog._create_hive_client(properties)
assert mock_hive_client.call_count == 2
+
+def test_list_views(hive_table: HiveTable) -> None:
Review Comment:
This test sets specific parameters then test those parameters.
I think creating an integration test using the HMS would be useful. That way
we can test the actual parameter for HMS
##########
pyiceberg/catalog/hive.py:
##########
@@ -407,7 +407,30 @@ def register_table(self, identifier: Union[str,
Identifier], metadata_location:
raise NotImplementedError
def list_views(self, namespace: Union[str, Identifier]) ->
List[Identifier]:
- raise NotImplementedError
+ """List Iceberg views under the given namespace in the catalog.
+
+ When the database doesn't exist, it will just return an empty list.
+
+ Args:
+ namespace: Database to list.
+
+ Returns:
+ List[Identifier]: List of view identifiers.
+
+ Raises:
+ NoSuchNamespaceError: If a namespace with the given name does not
exist, or the identifier is invalid.
+ """
+ database_name = self.identifier_to_database(namespace,
NoSuchNamespaceError)
+ with self._client as open_client:
+ return [
+ (database_name, view.tableName)
+ for view in open_client.get_table_objects_by_name(
Review Comment:
is this right? or is there a "view" equivalent function
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
