Copilot commented on code in PR #3032:
URL: https://github.com/apache/iceberg-rust/pull/3032#discussion_r3830618360
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -268,6 +305,62 @@ pub struct SqlCatalog {
sql_bind_style: SqlBindStyle,
runtime: Runtime,
kms_client: Option<Arc<dyn KeyManagementClient>>,
+ schema_version: SchemaVersion,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, strum::EnumString, strum::Display)]
+#[strum(ascii_case_insensitive)]
+/// Schema version of the `iceberg_tables` catalog table.
+pub enum SchemaVersion {
+ /// Original schema without the `iceberg_type` column.
+ V0,
+ /// Extended schema with the `iceberg_type` column for view support.
+ V1,
+}
+
+impl SchemaVersion {
+ /// Detect the schema version of an existing catalog table by
introspecting its columns.
+ async fn detect(pool: &AnyPool) -> Result<Self> {
+ let catalog_table_description = pool
+ .describe(&format!("SELECT * FROM {CATALOG_TABLE_NAME}"))
+ .await
+ .map_err(from_sqlx_error)?;
+
+ let has_type_column = catalog_table_description
+ .columns()
+ .iter()
+ .any(|column| column.name() == CATALOG_FIELD_RECORD_TYPE);
+
+ Ok(if has_type_column {
+ SchemaVersion::V1
+ } else {
+ SchemaVersion::V0
+ })
+ }
+
+ /// The trailing SQL `AND` clause used to exclude view rows when querying
for tables.
+ ///
+ /// `V1` schemas carry an `iceberg_type` column, so table rows are those
tagged `TABLE`
+ /// (or `NULL`, for rows written before the column existed). `V0` schemas
have no such
+ /// column, so no filter is applied.
+ fn record_type_filter(self) -> &'static str {
+ match self {
+ SchemaVersion::V1 => "AND (iceberg_type = 'TABLE' OR iceberg_type
IS NULL)",
+ SchemaVersion::V0 => "",
+ }
Review Comment:
`SchemaVersion::V0` only changes *read* queries via `record_type_filter()`,
but `create_table` and `register_table` still `INSERT` into `iceberg_tables`
with the `iceberg_type` column unconditionally (see `catalog.rs:1031-1034` and
`catalog.rs:1103-1107`). On a real V0 catalog table (no `iceberg_type` column)
this will still fail with "column iceberg_type does not exist" when
creating/registering tables. Inserts should omit the record-type column/value
when `schema_version == V0`, and only include it for V1.
--
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]
