danielcweeks commented on code in PR #9487:
URL: https://github.com/apache/iceberg/pull/9487#discussion_r1490219293
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java:
##########
@@ -81,6 +87,7 @@ public class JdbcCatalog extends BaseMetastoreCatalog
private final Function<Map<String, String>, JdbcClientPool>
clientPoolBuilder;
private final boolean initializeCatalogTables;
private CloseableGroup closeableGroup;
+ private boolean isNewSqlSchema = true;
Review Comment:
This concerns me just a little since it presupposes that we won't have any
additional schema migrations. As much as it pains me, we might want to track
the schema version with a table or at least infer a version based on the
changes we're making for views.
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java:
##########
@@ -81,85 +109,242 @@ final class JdbcUtil {
+ TABLE_NAME
+ ")"
+ ")";
- static final String GET_TABLE_SQL =
+ static final String UPDATE_CATALOG_SQL =
+ "ALTER TABLE " + CATALOG_TABLE_VIEW_NAME + " ADD COLUMN " + TYPE + "
VARCHAR(5)";
+
+ private static final String GET_VIEW_SQL =
"SELECT * FROM "
- + CATALOG_TABLE_NAME
+ + CATALOG_TABLE_VIEW_NAME
+ " WHERE "
+ CATALOG_NAME
+ " = ? AND "
+ TABLE_NAMESPACE
+ " = ? AND "
+ TABLE_NAME
- + " = ? ";
- static final String LIST_TABLES_SQL =
+ + " = ? AND "
+ + TYPE
+ + " = "
+ + "'"
+ + VIEW_RECORD_TYPE
+ + "'";
+ private static final String GET_TABLE_SQL =
"SELECT * FROM "
- + CATALOG_TABLE_NAME
+ + CATALOG_TABLE_VIEW_NAME
+ " WHERE "
+ CATALOG_NAME
+ " = ? AND "
+ TABLE_NAMESPACE
+ + " = ? AND "
+ + TABLE_NAME
+ + " = ? AND ("
+ + TYPE
+ + " = "
+ + "'"
+ + TABLE_RECORD_TYPE
+ + "'"
+ + " OR "
+ + TYPE
+ + " IS NULL)";
+ private static final String OLD_GET_TABLE_SQL =
Review Comment:
Same here, switch `OLD` for `V0`
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java:
##########
@@ -25,31 +25,57 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
+import org.apache.iceberg.BaseMetastoreTableOperations;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.relocated.com.google.common.base.Joiner;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.base.Splitter;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
final class JdbcUtil {
// property to control strict-mode (aka check if namespace exists when
creating a table)
static final String STRICT_MODE_PROPERTY = JdbcCatalog.PROPERTY_PREFIX +
"strict-mode";
+ // property to control if we add view support to the existing database
+ static final String ADD_VIEW_SUPPORT_PROPERTY = JdbcCatalog.PROPERTY_PREFIX
+ "add-view-support";
- // Catalog Table
- static final String CATALOG_TABLE_NAME = "iceberg_tables";
+ // Catalog Table & View
+ static final String CATALOG_TABLE_VIEW_NAME = "iceberg_tables";
static final String CATALOG_NAME = "catalog_name";
- static final String TABLE_NAMESPACE = "table_namespace";
static final String TABLE_NAME = "table_name";
+ static final String TABLE_NAMESPACE = "table_namespace";
+ static final String TYPE = "iceberg_type";
+ static final String TABLE_RECORD_TYPE = "TABLE";
+ static final String VIEW_RECORD_TYPE = "VIEW";
- static final String DO_COMMIT_SQL =
+ private static final String DO_COMMIT_SQL =
+ "UPDATE "
+ + CATALOG_TABLE_VIEW_NAME
+ + " SET "
+ + JdbcTableOperations.METADATA_LOCATION_PROP
+ + " = ? , "
+ + JdbcTableOperations.PREVIOUS_METADATA_LOCATION_PROP
+ + " = ?"
+ + " WHERE "
+ + CATALOG_NAME
+ + " = ? AND "
+ + TABLE_NAMESPACE
+ + " = ? AND "
+ + TABLE_NAME
+ + " = ? AND "
+ + JdbcTableOperations.METADATA_LOCATION_PROP
+ + " = ? AND "
+ + TYPE
+ + " = ?";
+ private static final String OLD_DO_COMMIT_SQL =
Review Comment:
Schema versioning here, e.g. `V0_DO_COMMIT_SQL` as opposed to `OLD` which
feels very contextual.
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java:
##########
@@ -52,26 +49,29 @@ class JdbcTableOperations extends
BaseMetastoreTableOperations {
private final FileIO fileIO;
private final JdbcClientPool connections;
private final Map<String, String> catalogProperties;
+ private final boolean isNewSqlSchema;
Review Comment:
Same thing here, we should probably have something (enum/class) to represent
the backing schema version. A boolean limits future evolution.
--
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
