abnobdoss commented on code in PR #3415:
URL: https://github.com/apache/iceberg-python/pull/3415#discussion_r3456585248
##########
pyiceberg/catalog/__init__.py:
##########
@@ -744,6 +745,49 @@ def create_view(
ViewAlreadyExistsError: If a view with the name already exists.
"""
+ def create_sql_view(
+ self,
+ identifier: str | Identifier,
+ schema: Schema | pa.Schema,
+ dialect: str,
+ sql: str,
+ default_namespace: str | Identifier,
+ location: str | None = None,
+ properties: Properties = EMPTY_DICT,
+ default_catalog: str | None = None,
+ ) -> View:
+ """Create a view.
+
+ Args:
+ identifier (str | Identifier): View identifier.
+ schema (Schema): View's schema.
Review Comment:
```suggestion
schema (Schema | pa.Schema): View's schema.
```
##########
pyiceberg/catalog/__init__.py:
##########
@@ -744,6 +745,49 @@ def create_view(
ViewAlreadyExistsError: If a view with the name already exists.
"""
+ def create_sql_view(
+ self,
+ identifier: str | Identifier,
+ schema: Schema | pa.Schema,
+ dialect: str,
+ sql: str,
+ default_namespace: str | Identifier,
+ location: str | None = None,
+ properties: Properties = EMPTY_DICT,
+ default_catalog: str | None = None,
+ ) -> View:
+ """Create a view.
Review Comment:
Should this description be different from create_view?
##########
pyiceberg/catalog/__init__.py:
##########
@@ -744,6 +745,49 @@ def create_view(
ViewAlreadyExistsError: If a view with the name already exists.
"""
+ def create_sql_view(
+ self,
+ identifier: str | Identifier,
+ schema: Schema | pa.Schema,
+ dialect: str,
+ sql: str,
+ default_namespace: str | Identifier,
+ location: str | None = None,
+ properties: Properties = EMPTY_DICT,
+ default_catalog: str | None = None,
+ ) -> View:
+ """Create a view.
+
+ Args:
+ identifier (str | Identifier): View identifier.
+ schema (Schema): View's schema.
+ dialect (str): SQL dialect for the view.
+ sql (str): SQL for the view.
+ default_namespace (str | Identifier): Default namespace name.
+ location (str | None): Location for the view. Optional Argument.
+ properties (Properties): View properties that can be a string
based dictionary.
+ default_catalog (str | None): Default catalog name. Optional
Argument.
+
+ Returns:
+ View: the created view instance.
+
+ Raises:
+ ViewAlreadyExistsError: If a view with the name already exists.
+ """
+ iceberg_schema = self._convert_schema_if_needed(schema)
+ namespace_tuple = Catalog.identifier_to_tuple(default_namespace)
+
+ view_version = ViewVersion(
+ version_id=1,
+ schema_id=iceberg_schema.schema_id,
Review Comment:
is this schema_id discarded later in the self.create_view call later?
https://github.com/apache/iceberg-python/blob/c85c9eecb1b79cdc0acd4dc27983802788518aa3/pyiceberg/catalog/rest/__init__.py#L981
##########
tests/integration/test_catalog.py:
##########
@@ -653,6 +653,23 @@ def test_rest_create_view(
assert rest_catalog.load_view(identifier).schema() == view.schema()
[email protected]
+def test_rest_create_sql_view(
Review Comment:
The `create_sql_view` function has a few permutations of accepted values -
e.g. default_namespace accepts both `str` & `Identifier`. Could we expand the
test coverage to cover the various signature paths?
--
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]
