rdblue commented on code in PR #7921: URL: https://github.com/apache/iceberg/pull/7921#discussion_r1252384583
########## python/pyiceberg/catalog/jdbc.py: ########## @@ -0,0 +1,452 @@ +import sqlite3 +from typing import ( + Any, + Dict, + List, + Optional, + Set, + Union, +) +from urllib.parse import urlparse + +import psycopg2 as db +from psycopg2.errors import UniqueViolation +from psycopg2.extras import DictCursor + +from pyiceberg.catalog import ( + METADATA_LOCATION, + PREVIOUS_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 + +JDBC_URI = "uri" + +# Catalog tables +CATALOG_TABLE_NAME = "iceberg_tables" +CATALOG_NAME = "catalog_name" +TABLE_NAMESPACE = "table_namespace" +TABLE_NAME = "table_name" Review Comment: I was skeptical of these constants in the Java version and I'd probably just embed them in the SQL here. The parameterization seems difficult to work with for little gain. I can't imagine many people actually changing these constants. -- 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]
