[backup][YCQL] Incorrect column-ids in restored YCQL table if original table was altered.

[OlegLoginov]

Currently CatalogManager::CreateTable() sets column ids into 0,1,2,3..
Due to ALTER TABLE statements columns can be added/removed. (E.g. into 0,2,3 after ALTER TABLE t DROP col1;)
But on the backup-restore step RecreateTable() calls CreateTable(), which sets the column ids into '0,1,2' instead of using existing in the backup ids.
As result the restored table schema references to wrong columns.

Example. Source table:

ycqlsh:b> create table t (a int, b int, c int, h int primary key);
ycqlsh:b> insert into t (a,b,c,h) values (1,2,3,4);
ycqlsh:b> insert into t (a,b,c,h) values (5,6,7,8);
ycqlsh:b> select * from t;
 h | a | b | c
---+---+---+---
 4 | 1 | 2 | 3
 8 | 5 | 6 | 7
(2 rows)
ycqlsh:b> alter table t drop  a;
ycqlsh:b> select * from t;
 h | b | c
---+---+---
 4 | 2 | 3
 8 | 6 | 7
(2 rows)
ycqlsh:b> desc t
CREATE TABLE b.t (
    h int PRIMARY KEY,                <<< id=0
    b int,                            <<< id=2
    c int                             <<< id=3
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};

Restored table:

ycqlsh:b2> desc t
CREATE TABLE b2.t (
    h int PRIMARY KEY,             <<< id=0
    b int,                         <<< id=1   (wrong! must be 2)
    c int                          <<< id=2   (wrong! must be 3)
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};
ycqlsh:b2> select * from t;
 h | b | c
---+---+---
 4 | 1 | 2
 8 | 5 | 6
(2 rows)

[OlegLoginov]

Actually the backup is correct and all data is available. The bug is in restore-step.
Workaround: manually add needed columns back and rename other columns if needed.
In this example:

ycqlsh:b2> desc t
CREATE TABLE b2.t (
    h int PRIMARY KEY,             <<< id=0
    b int,                         <<< id=1   (wrong! must be 2)
    c int                          <<< id=2   (wrong! must be 3)
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};
ycqlsh:b2> select * from t;
 h | b | c
---+---+---
 4 | 1 | 2
 8 | 5 | 6
(2 rows)

Manual changes:

ycqlsh:b2> alter table t add c2 int;     <<< c2: id=3
ycqlsh:b2> select * from t;
 h | b | c | c2
---+---+---+----
 4 | 1 | 2 |  3
 8 | 5 | 6 |  7
(2 rows)
ycqlsh:b2> alter table t rename b to a;
ycqlsh:b2> alter table t rename c to b;
ycqlsh:b2> alter table t rename c2 to c;
ycqlsh:b2> select * from t;
 h | a | b | c
---+---+---+---
 4 | 1 | 2 | 3
 8 | 5 | 6 | 7
(2 rows)
ycqlsh:b2> alter table t drop  a;
ycqlsh:b2> select * from t;
 h | b | c
---+---+---
 4 | 2 | 3
 8 | 6 | 7
(2 rows)
ycqlsh:b2> desc b.t
CREATE TABLE b.t (
    h int PRIMARY KEY,
    b int,                         <<< id=2   OK
    c int                          <<< id=3   OK
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};

NOTE: the renaming workaround works only if the same column types were used.
For a case with different types - see below.

[OlegLoginov]

Issue is the source code:

1. CatalogManager::RecreateTable() - the code should be deleted:

  // Clear column IDs.
  for (int i = 0; i < schema->columns_size(); ++i) {
    DCHECK_NOTNULL(schema->mutable_columns(i))->clear_id();
  }

2. ValidateCreateTableSchema() - the check should be removed or updated:

  if (schema.has_column_ids()) {
    return SetupError(resp->mutable_error(), MasterErrorPB::INVALID_SCHEMA,
                      STATUS(InvalidArgument, "User requests should not have Column IDs"));
  }

3. In the CatalogManager::CreateTable() - fix schema setup to reuse old (provided by the caller) column ids:

  Schema schema = client_schema.CopyWithColumnIds(); 

use client_schema.CopyWithoutColumnIds(); if the schema has column ids.

[OlegLoginov]

Workaround for a case when different types were used.
It includes additional steps to restore correct column types.
The procedure:

1. Restore the table (ids are wrong).
2. Drop columns, add all columns back with correct order & correct types (column ids must be n,n+1,n+2,n+3 - correct order & names & types, but wrong id values).
3. Create the table backup.
4. Restore the second backup - the table is correct now (extra columns can be dropped).

Original table (b.t2):

ycqlsh:b> create table t2 (a int, b double, c text, h int primary key);
ycqlsh:b> insert into t2 (a,b,c,h) values (1,3.14,'qqq',8);
ycqlsh:b> insert into t2 (a,b,c,h) values (2,4.14,'www',9);
ycqlsh:b> select * from t2;
 h | a | b    | c
---+---+------+-----
 9 | 2 | 4.14 | www
 8 | 1 | 3.14 | qqq
(2 rows)
ycqlsh:b> alter table t2 drop a;
ycqlsh:b> select * from t2;
 h | b    | c
---+------+-----
 9 | 4.14 | www
 8 | 3.14 | qqq
(2 rows)

Restore the table from the backup (into b2.t2), and change the columns:

ycqlsh:b2> desc t2                <<<<<<<<<<<< restored b2.t2
CREATE TABLE b2.t2 (
    h int PRIMARY KEY,
    b double,                    <<<<<<<< bad column id
    c text                       <<<<<<<< bad column id
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};
ycqlsh:b2> alter table t2 drop c;
ycqlsh:b2> alter table t2 drop b;
ycqlsh:b2> alter table t2 add a int;
ycqlsh:b2> alter table t2 add b double;
ycqlsh:b2> alter table t2 add c text;
ycqlsh:b2> desc t2
CREATE TABLE b2.t2 (
    h int PRIMARY KEY,
    a int,                  <<<< correct column & type (id is still wong)
    b double,               <<<< correct column & type (id is still wong)
    c text                  <<<< correct column & type (id is still wong)
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};

Create a new backup of the table b2.t2 & restore it back into the table b3.t2.

ycqlsh:b3> desc t2
CREATE TABLE b3.t2 (
    h int PRIMARY KEY,
    a int,              <<<<< correct name & type & id = 1
    b double,           <<<<< correct name & type & id = 2
    c text              <<<<< correct name & type & id = 3
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};
ycqlsh:b3> select * from t2;
 h | a | b    | c
---+---+------+-----
 9 | 2 | 4.14 | www
 8 | 1 | 3.14 | qqq
(2 rows)

[OlegLoginov]

Fixed by the commit above: fa1ba0a