cccs-eric commented on code in PR #7921:
URL: https://github.com/apache/iceberg/pull/7921#discussion_r1268311333


##########
python/pyiceberg/catalog/sql.py:
##########
@@ -0,0 +1,441 @@
+from typing import (
+    List,
+    Optional,
+    Set,
+    Union,
+)
+
+from sqlalchemy import (
+    String,
+    case,
+    create_engine,
+    delete,
+    select,
+    union,
+    update,
+)
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm import (
+    DeclarativeBase,
+    Mapped,
+    MappedAsDataclass,
+    Session,
+    mapped_column,
+)
+
+from pyiceberg.catalog import (
+    METADATA_LOCATION,
+    Catalog,
+    Identifier,
+    Properties,
+    PropertiesUpdateSummary,
+)
+from pyiceberg.exceptions import (
+    NamespaceAlreadyExistsError,
+    NamespaceNotEmptyError,
+    NoSuchNamespaceError,
+    NoSuchPropertyException,
+    NoSuchTableError,
+    TableAlreadyExistsError,
+)
+from pyiceberg.io import load_file_io
+from pyiceberg.partitioning import UNPARTITIONED_PARTITION_SPEC, PartitionSpec
+from pyiceberg.schema import Schema
+from pyiceberg.serializers import FromInputFile
+from pyiceberg.table import Table
+from pyiceberg.table.metadata import new_table_metadata
+from pyiceberg.table.sorting import UNSORTED_SORT_ORDER, SortOrder
+from pyiceberg.typedef import EMPTY_DICT
+
+
+class SQLCatalogBase(MappedAsDataclass, DeclarativeBase):
+    pass
+
+
+class IcebergTables(SQLCatalogBase):
+    __tablename__ = "iceberg_tables"
+
+    catalog_name: Mapped[str] = mapped_column(String(255), nullable=False, 
primary_key=True)
+    table_namespace: Mapped[str] = mapped_column(String(255), nullable=False, 
primary_key=True)
+    table_name: Mapped[str] = mapped_column(String(255), nullable=False, 
primary_key=True)
+    metadata_location: Mapped[str] = mapped_column(String(1000), nullable=True)
+    previous_metadata_location: Mapped[str] = mapped_column(String(1000), 
nullable=True)
+
+
+class IcebergNamespaceProperties(SQLCatalogBase):
+    __tablename__ = "iceberg_namespace_properties"
+    # Catalog minimum Namespace Properties
+    NAMESPACE_MINIMAL_PROPERTIES = {"exists": "true"}
+
+    catalog_name: Mapped[str] = mapped_column(String(255), nullable=False, 
primary_key=True)
+    namespace: Mapped[str] = mapped_column(String(255), nullable=False, 
primary_key=True)
+    property_key: Mapped[str] = mapped_column(String(255), nullable=False, 
primary_key=True)
+    property_value: Mapped[str] = mapped_column(String(1000), nullable=True)

Review Comment:
   Ok, so I will make both `property_key` and `property_value` NOT NULLABLE.  
Not sure if this case would happen, but you could see a SQL-based catalog being 
created by pyiceberg, then used by JDBCCatalog.  Since the schema in the Java 
implementation allows `null` properties, I don't know if the Java 
implementation would allow a user to add a null key or value.  This is an edge 
case, all I'm saying is a user could expect a null key or value to work when in 
Java...  I think the JDBCCatalog code should be changed to not allow nulls.



-- 
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]

Reply via email to