badalprasadsingh commented on code in PR #1244:
URL: https://github.com/apache/iceberg-go/pull/1244#discussion_r3530950273
##########
catalog/sql/sql_test.go:
##########
@@ -322,6 +336,358 @@ func (s *SqliteCatalogTestSuite)
TestCreationAllTablesExist() {
s.confirmTablesExist(sqldb)
}
+func (s *SqliteCatalogTestSuite) TestV0SchemaFromIcebergIsMigrated() {
+ sqldb := s.getDB()
+ s.confirmNoTables(sqldb)
+
+ _, err := sqldb.Exec(`CREATE TABLE "iceberg_tables" (
+ "catalog_name" VARCHAR NOT NULL,
+ "table_namespace" VARCHAR NOT NULL,
+ "table_name" VARCHAR NOT NULL,
+ "metadata_location" VARCHAR,
+ "previous_metadata_location" VARCHAR,
+ PRIMARY KEY ("catalog_name", "table_namespace", "table_name"))`)
+ s.Require().NoError(err)
+ _, err = sqldb.Exec(`CREATE TABLE "iceberg_namespace_properties" (
+ "catalog_name" VARCHAR NOT NULL,
+ "namespace" VARCHAR NOT NULL,
+ "property_key" VARCHAR NOT NULL,
+ "property_value" VARCHAR,
+ PRIMARY KEY ("catalog_name", "namespace", "property_key"))`)
+ s.Require().NoError(err)
+
+ const (
+ catName = "default"
+ nsName = "test_v0_ns"
+ tblName = "test_v0_tbl"
+ metaLoc = "file:///does/not/matter/00000-v0.metadata.json"
+ )
+ _, err = sqldb.Exec(
+ `INSERT INTO "iceberg_namespace_properties" `+
+ `("catalog_name", "namespace", "property_key",
"property_value") VALUES (?, ?, ?, ?)`,
+ catName, nsName, "exists", "true")
+ s.Require().NoError(err)
+ _, err = sqldb.Exec(
+ `INSERT INTO "iceberg_tables" `+
+ `("catalog_name", "table_namespace", "table_name",
"metadata_location") VALUES (?, ?, ?, ?)`,
+ catName, nsName, tblName, metaLoc)
+ s.Require().NoError(err)
+
+ _ = s.loadCatalogWithV1Migration()
+
+ columnPresent := func() bool {
+ var present int
+ qErr := sqldb.QueryRow(
+ `SELECT 1 FROM pragma_table_info('iceberg_tables')
WHERE name = 'iceberg_type'`,
+ ).Scan(&present)
+ s.Require().NoError(qErr, "migration must add iceberg_type
column")
+
+ return present == 1
+ }
+ s.True(columnPresent(), "migration must add iceberg_type column")
+
+ rowType := func() sql.NullString {
+ var got sql.NullString
+ qErr := sqldb.QueryRow(
+ `SELECT iceberg_type FROM iceberg_tables WHERE
catalog_name = ? AND table_namespace = ? AND table_name = ?`,
+ catName, nsName, tblName,
+ ).Scan(&got)
+ s.Require().NoError(qErr)
+
+ return got
+ }
+ s.False(rowType().Valid, "pre-V0 row should keep iceberg_type IS NULL
after migration")
+
+ // Re-opening with the same V1 opt-in must be idempotent: the column
stays
+ // present and the legacy row's iceberg_type is left NULL (not re-added
or stamped).
+ _ = s.loadCatalogWithV1Migration()
+ s.True(columnPresent(), "column must remain present after a second
open")
+ s.False(rowType().Valid, "legacy row must remain NULL after a second
open")
+}
+
+func (s *SqliteCatalogTestSuite) TestV0SchemaNotMigratedWithoutOptIn() {
+ sqldb := s.getDB()
+ s.confirmNoTables(sqldb)
+
+ _, err := sqldb.Exec(`CREATE TABLE "iceberg_tables" (
+ "catalog_name" VARCHAR NOT NULL,
+ "table_namespace" VARCHAR NOT NULL,
+ "table_name" VARCHAR NOT NULL,
+ "metadata_location" VARCHAR,
+ "previous_metadata_location" VARCHAR,
+ PRIMARY KEY ("catalog_name", "table_namespace", "table_name"))`)
+ s.Require().NoError(err)
+ _, err = sqldb.Exec(`CREATE TABLE "iceberg_namespace_properties" (
+ "catalog_name" VARCHAR NOT NULL,
+ "namespace" VARCHAR NOT NULL,
+ "property_key" VARCHAR NOT NULL,
+ "property_value" VARCHAR,
+ PRIMARY KEY ("catalog_name", "namespace", "property_key"))`)
+ s.Require().NoError(err)
+
+ // Without jdbc.schema-version=V1, opening a legacy V0 catalog must NOT
issue
+ // the one-way ALTER; the column stays absent.
+ _ = s.loadCatalogForTableCreation()
+
+ var present int
+ err = sqldb.QueryRow(
+ `SELECT 1 FROM pragma_table_info('iceberg_tables') WHERE name =
'iceberg_type'`,
+ ).Scan(&present)
+ s.ErrorIs(err, sql.ErrNoRows, "iceberg_type column must not be added
without the V1 opt-in")
+}
+
+func (s *SqliteCatalogTestSuite)
TestV0SchemaListAndDropAcceptNullIcebergType() {
+ sqldb := s.getDB()
+ s.confirmNoTables(sqldb)
+
+ _, err := sqldb.Exec(`CREATE TABLE "iceberg_tables" (
+ "catalog_name" VARCHAR NOT NULL,
+ "table_namespace" VARCHAR NOT NULL,
+ "table_name" VARCHAR NOT NULL,
+ "metadata_location" VARCHAR,
+ "previous_metadata_location" VARCHAR,
+ PRIMARY KEY ("catalog_name", "table_namespace", "table_name"))`)
+ s.Require().NoError(err)
+ _, err = sqldb.Exec(`CREATE TABLE "iceberg_namespace_properties" (
+ "catalog_name" VARCHAR NOT NULL,
+ "namespace" VARCHAR NOT NULL,
+ "property_key" VARCHAR NOT NULL,
+ "property_value" VARCHAR,
+ PRIMARY KEY ("catalog_name", "namespace", "property_key"))`)
+ s.Require().NoError(err)
+
+ const (
+ catName = "default"
+ nsName = "legacy_ns"
+ tblName = "legacy_tbl"
+ metaLoc = "file:///legacy/00000-v0.metadata.json"
+ )
+ _, err = sqldb.Exec(
+ `INSERT INTO "iceberg_namespace_properties" `+
+ `("catalog_name", "namespace", "property_key",
"property_value") VALUES (?, ?, ?, ?)`,
+ catName, nsName, "exists", "true")
+ s.Require().NoError(err)
+ _, err = sqldb.Exec(
+ `INSERT INTO "iceberg_tables" `+
+ `("catalog_name", "table_namespace", "table_name",
"metadata_location") VALUES (?, ?, ?, ?)`,
+ catName, nsName, tblName, metaLoc)
+ s.Require().NoError(err)
+
+ // Migrate to V1: this adds the iceberg_type column and leaves the
legacy row
+ // with iceberg_type IS NULL, which the read path must still tolerate.
+ cat := s.loadCatalogWithV1Migration()
+ ctx := context.Background()
+
+ var listed []table.Identifier
+ for ident, err := range cat.ListTables(ctx, table.Identifier{nsName}) {
+ s.Require().NoError(err)
+ listed = append(listed, ident)
+ }
+ s.Require().Len(listed, 1, "ListTables must return rows with
iceberg_type IS NULL")
+ s.Equal(tblName, listed[0][len(listed[0])-1])
+
+ s.Require().NoError(cat.DropTable(ctx, table.Identifier{nsName,
tblName}))
+
+ var remaining int
+ err = sqldb.QueryRow(
+ `SELECT COUNT(*) FROM iceberg_tables WHERE catalog_name = ? AND
table_namespace = ? AND table_name = ?`,
+ catName, nsName, tblName,
+ ).Scan(&remaining)
+ s.Require().NoError(err)
+ s.Equal(0, remaining, "DropTable must delete rows with iceberg_type IS
NULL")
+}
+
+func (s *SqliteCatalogTestSuite)
TestV0SchemaLoadCommitRenameAcceptNullIcebergType() {
+ sqldb := s.getDB()
+
+ cat := s.getCatalogSqlite()
+ ctx := context.Background()
+
+ // This is a proxy for a genuine V0 row: we create the row at V1 (so the
Review Comment:
Done.
You are right, added the test for it.
--
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]
