diff --git a/ent/src/yb/master/catalog_manager_ent.cc b/ent/src/yb/master/catalog_manager_ent.cc index 4cb3a6d96575..dd535addbe32 100644 --- a/ent/src/yb/master/catalog_manager_ent.cc +++ b/ent/src/yb/master/catalog_manager_ent.cc @@ -943,13 +943,9 @@ Status CatalogManager::RecreateTable(const NamespaceId& new_namespace_id, *req.mutable_partition_schema() = meta.partition_schema(); *req.mutable_replication_info() = meta.replication_info(); - // Clear column IDs. SchemaPB* const schema = req.mutable_schema(); - schema->mutable_table_properties()->set_num_tablets(table_data->num_tablets); *schema = meta.schema(); - for (int i = 0; i < schema->columns_size(); ++i) { - DCHECK_NOTNULL(schema->mutable_columns(i))->clear_id(); - } + schema->mutable_table_properties()->set_num_tablets(table_data->num_tablets); // Setup Index info. if (table_data->is_index()) { diff --git a/java/yb-cql/src/test/java/org/yb/cql/TestYbBackup.java b/java/yb-cql/src/test/java/org/yb/cql/TestYbBackup.java index f9e8e987b265..a2a0bf0ee389 100644 --- a/java/yb-cql/src/test/java/org/yb/cql/TestYbBackup.java +++ b/java/yb-cql/src/test/java/org/yb/cql/TestYbBackup.java @@ -591,4 +591,34 @@ public void testYCQLBackupWithJsonIndex() throws Exception { "Row[1, {\"a\":{\"b\":\"b1\"},\"c\":1}]"); assertQuery("select * from ks7.test_json_tbl where h=9999;", ""); } + + @Test + public void testAlteredYCQLTableBackup() throws Exception { + session.execute("create table test_tbl (h int primary key, a int, b float) " + + "with transactions = { 'enabled' : true }"); + + for (int i = 1; i <= 2000; ++i) { + session.execute("insert into test_tbl (h, a, b) values" + + " (" + String.valueOf(i) + // h + ", " + String.valueOf(100 + i) + // a + ", " + String.valueOf(2.14 + (float)i) + ")"); // b + } + + session.execute("alter table test_tbl drop a;"); + runYbBackupCreate("--keyspace", DEFAULT_TEST_KEYSPACE, "--table", "test_tbl"); + session.execute("insert into test_tbl (h, b) values (9999, 8.9)"); + + runYbBackupRestore("--keyspace", "ks1"); + assertQuery("select * from " + DEFAULT_TEST_KEYSPACE + ".test_tbl where h=1", "Row[1, 3.14]"); + assertQuery("select * from " + DEFAULT_TEST_KEYSPACE + ".test_tbl where h=2000", + "Row[2000, 2002.14]"); + assertQuery("select * from " + DEFAULT_TEST_KEYSPACE + ".test_tbl where h=9999", + "Row[9999, 8.9]"); + + assertQuery("select * from ks1.test_tbl where h=1", "Row[1, 3.14]"); + assertQuery("select b from ks1.test_tbl where h=1", "Row[3.14]"); + assertQuery("select * from ks1.test_tbl where h=2000", "Row[2000, 2002.14]"); + assertQuery("select h from ks1.test_tbl where h=2000", "Row[2000]"); + assertQuery("select * from ks1.test_tbl where h=9999", ""); + } } diff --git a/src/yb/common/schema.cc b/src/yb/common/schema.cc index 6549fc4956f3..b1316d9a7249 100644 --- a/src/yb/common/schema.cc +++ b/src/yb/common/schema.cc @@ -252,6 +252,19 @@ void Schema::swap(Schema& other) { DCHECK(cotable_id_.IsNil() || pgtable_id_ == 0); } +void Schema::ResetColumnIds(const vector& ids) { + // Initialize IDs mapping. + col_ids_ = ids; + id_to_index_.clear(); + max_col_id_ = 0; + for (int i = 0; i < ids.size(); ++i) { + if (ids[i] > max_col_id_) { + max_col_id_ = ids[i]; + } + id_to_index_.set(ids[i], i); + } +} + Status Schema::Reset(const vector& cols, const vector& ids, int key_columns, @@ -339,15 +352,7 @@ Status Schema::Reset(const vector& cols, col_offsets_.push_back(off); // Initialize IDs mapping - col_ids_ = ids; - id_to_index_.clear(); - max_col_id_ = 0; - for (int i = 0; i < ids.size(); ++i) { - if (ids[i] > max_col_id_) { - max_col_id_ = ids[i]; - } - id_to_index_.set(ids[i], i); - } + ResetColumnIds(ids); // Ensure clustering columns have a default sorting type of 'ASC' if not specified. for (int i = num_hash_key_columns_; i < num_key_columns(); i++) { @@ -392,13 +397,25 @@ Status Schema::CreateProjectionByIdsIgnoreMissing(const std::vector& c return out->Reset(cols, filtered_col_ids, 0, TableProperties(), cotable_id_, pgtable_id_); } -Schema Schema::CopyWithColumnIds() const { - CHECK(!has_column_ids()); +namespace { +vector DefaultColumnIds(size_t num_columns) { vector ids; - for (int32_t i = 0; i < num_columns(); i++) { + for (int32_t i = 0; i < num_columns; ++i) { ids.push_back(ColumnId(kFirstColumnId + i)); } - return Schema(cols_, ids, num_key_columns_, table_properties_, cotable_id_, pgtable_id_); + return ids; +} +} // namespace + +Schema Schema::CopyWithColumnIds() const { + CHECK(!has_column_ids()); + return Schema(cols_, DefaultColumnIds(num_columns()), + num_key_columns_, table_properties_, cotable_id_, pgtable_id_); +} + +void Schema::InitColumnIdsByDefault() { + CHECK(!has_column_ids()); + ResetColumnIds(DefaultColumnIds(cols_.size())); } Schema Schema::CopyWithoutColumnIds() const { diff --git a/src/yb/common/schema.h b/src/yb/common/schema.h index dc14c616f0f7..05959cf1302a 100644 --- a/src/yb/common/schema.h +++ b/src/yb/common/schema.h @@ -950,6 +950,10 @@ class Schema { // Requires that this schema has no column IDs. Schema CopyWithColumnIds() const; + // Initialize column IDs by default values. + // Requires that this schema has no column IDs. + void InitColumnIdsByDefault(); + // Return a new Schema which is the same as this one, but without any column // IDs assigned. // @@ -1133,6 +1137,8 @@ class Schema { private: + void ResetColumnIds(const vector& ids); + // Return a stringified version of the first 'num_columns' columns of the // row. template diff --git a/src/yb/master/catalog_manager.cc b/src/yb/master/catalog_manager.cc index 24600feadbc6..f179b9e949f4 100644 --- a/src/yb/master/catalog_manager.cc +++ b/src/yb/master/catalog_manager.cc @@ -1838,10 +1838,6 @@ Status CatalogManager::DeleteTablet( namespace { CHECKED_STATUS ValidateCreateTableSchema(const Schema& schema, CreateTableResponsePB* resp) { - if (schema.has_column_ids()) { - return SetupError(resp->mutable_error(), MasterErrorPB::INVALID_SCHEMA, - STATUS(InvalidArgument, "User requests should not have Column IDs")); - } if (schema.num_key_columns() <= 0) { return SetupError(resp->mutable_error(), MasterErrorPB::INVALID_SCHEMA, STATUS(InvalidArgument, "Must specify at least one key column")); @@ -1867,16 +1863,14 @@ Status CatalogManager::CreatePgsqlSysTable(const CreateTableRequestPB* req, const NamespaceId& namespace_id = ns->id(); const NamespaceName& namespace_name = ns->name(); - Schema schema; Schema client_schema; RETURN_NOT_OK(SchemaFromPB(req->schema(), &client_schema)); // If the schema contains column ids, we are copying a Postgres table from one namespace to - // another. In that case, just use the schema as-is. Otherwise, validate the schema. - if (client_schema.has_column_ids()) { - schema = std::move(client_schema); - } else { - RETURN_NOT_OK(ValidateCreateTableSchema(client_schema, resp)); - schema = client_schema.CopyWithColumnIds(); + // another. Anyway, validate the schema. + RETURN_NOT_OK(ValidateCreateTableSchema(client_schema, resp)); + Schema schema = std::move(client_schema); + if (!schema.has_column_ids()) { + schema.InitColumnIdsByDefault(); } schema.mutable_table_properties()->set_is_ysql_catalog_table(true); @@ -2210,7 +2204,13 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req, } // TODO (ENG-1860) The referenced namespace and types retrieved/checked above could be deleted // some time between this point and table creation below. - Schema schema = client_schema.CopyWithColumnIds(); + Schema schema = std::move(client_schema); + // Usually the column ids are available if it's called on the backup-restoring code path + // (from CatalogManager::RecreateTable). Else the column ids must be empty in the client schema. + if (!schema.has_column_ids()) { + schema.InitColumnIdsByDefault(); + } + if (schema.table_properties().HasCopartitionTableId()) { return CreateCopartitionedTable(req, resp, rpc, schema, ns); }