feat(query): create drop inverted index

[b41sh]

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

• Parser support create/drop inverted index
• TableMeta adds indexes field
• Add create_table_index and drop_table_index api

• part of Tracking: Inverted index #14825

Tests

• Unit Test
• Logic Test
• Benchmark Test
• No Test - Explain why

Type of change

• Bug Fix (non-breaking change which fixes an issue)
• New Feature (non-breaking change which adds functionality)
• Breaking Change (fix or feature that could cause existing functionality not to work as expected)
• Documentation Update
• Refactoring
• Performance Improvement
• Other (please describe):

---

This change is 

[drmingdrmer]

Reviewed 14 of 43 files at r1.
Reviewable status: 14 of 44 files reviewed, 1 unresolved discussion (waiting on @b41sh and @sundy-li)

---

src/meta/api/src/schema_api_impl.rs line 4013 at r1 (raw file):

            UnknownIndex::new(&tenant_index.index_name, "drop index with different type"),
        )));
    }

An index is uniquely identified by index_type at line 3985, does it have to check index_type?

Code quote:

    if index_type != &index_meta.index_type {
        return Err(KVAppError::AppError(AppError::UnknownIndex(
            UnknownIndex::new(&tenant_index.index_name, "drop index with different type"),
        )));
    }

[b41sh]

Reviewed 14 of 43 files at r1.
Reviewable status: 14 of 44 files reviewed, 1 unresolved discussion (waiting on @b41sh and @sundy-li)

src/meta/api/src/schema_api_impl.rs line 4013 at r1 (raw file):

            UnknownIndex::new(&tenant_index.index_name, "drop index with different type"),
        )));
    }

An index is uniquely identified by index_type at line 3985, does it have to check index_type?

Code quote:

    if index_type != &index_meta.index_type {
        return Err(KVAppError::AppError(AppError::UnknownIndex(
            UnknownIndex::new(&tenant_index.index_name, "drop index with different type"),
        )));
    }

The aggregating index and the inverted index have different drop syntaxes,

DROP AGGREGATING INDEX idx1;
DROP INVERTED INDEX idx1;

The main reason for checking the index type here is to avoid using the wrong drop syntax, for example:

CREATE AGGREGATING INDEX my_agg_index AS SELECT MIN(a), MAX(c) FROM agg;
DROP INVERTED INDEX my_agg_index;

[drmingdrmer]

The aggregating index and the inverted index have different drop syntaxes,

DROP AGGREGATING INDEX idx1;
DROP INVERTED INDEX idx1;

The main reason for checking the index type here is to avoid using the wrong drop syntax, for example:

CREATE AGGREGATING INDEX my_agg_index AS SELECT MIN(a), MAX(c) FROM agg;
DROP INVERTED INDEX my_agg_index;

Is it allowed for an aggregating index and an inverted index to have same index name? For example:

CREATE AGGREGATING INDEX aaa ...;
CREATE INVERTED INDEX aaa ...;

[b41sh]

The aggregating index and the inverted index have different drop syntaxes,

DROP AGGREGATING INDEX idx1;
DROP INVERTED INDEX idx1;

The main reason for checking the index type here is to avoid using the wrong drop syntax, for example:

CREATE AGGREGATING INDEX my_agg_index AS SELECT MIN(a), MAX(c) FROM agg;
DROP INVERTED INDEX my_agg_index;

Is it allowed for an aggregating index and an inverted index to have same index name? For example:

CREATE AGGREGATING INDEX aaa ...;
CREATE INVERTED INDEX aaa ...;

Same name is not allowed.

[drmingdrmer]

Reviewed all commit messages.
Reviewable status: 14 of 45 files reviewed, 2 unresolved discussions (waiting on @b41sh and @sundy-li)

---

src/meta/app/src/schema/index.rs line 96 at r3 (raw file):

    AGGREGATING = 1,
    JOIN = 2,
    INVERTED = 3,

Adding a variant is a incompatible change. In a cluster of new-query and old-query running together, the old-query that can not read INVERTED will return an error

[sundy-li]

Adding a variant is a incompatible change. In a cluster of new-query and old-query running together, the old-query that can not read INVERTED will return an error

Seems reasonable, new-query should remove this new meta to downgrade.

[drmingdrmer]

Reviewed all commit messages.
Reviewable status: 14 of 45 files reviewed, 2 unresolved discussions (waiting on @b41sh and @sundy-li)

src/meta/app/src/schema/index.rs line 96 at r3 (raw file):

    AGGREGATING = 1,
    JOIN = 2,
    INVERTED = 3,

Adding a variant is a incompatible change. In a cluster of new-query and old-query running together, the old-query that can not read INVERTED will return an error

@TCeason had solved a similar compatibility issue like this. What's the correct way for adding a enum variant?

[b41sh]

Adding a variant is a incompatible change. In a cluster of new-query and old-query running together, the old-query that can not read INVERTED will return an error

@TCeason had solved a similar compatibility issue like this. What's the correct way for adding a enum variant?

We can store the data for the inverted index separately, but this will bring some duplicate code, is there a better way to solve this problem?

[TCeason]

We can store the data for the inverted index separately, but this will bring some duplicate code, is there a better way to solve this problem?

PriviligeType is bitflag. Use from_bits_truncate.

let privileges =
            BitFlags::<mt::principal::UserPrivilegeType, u64>::from_bits_truncate(p.privileges);

[TCeason]

Reviewed all commit messages.
Reviewable status: 14 of 45 files reviewed, 2 unresolved discussions (waiting on @b41sh and @sundy-li)

src/meta/app/src/schema/index.rs line 96 at r3 (raw file):

    AGGREGATING = 1,
    JOIN = 2,
    INVERTED = 3,

Adding a variant is a incompatible change. In a cluster of new-query and old-query running together, the old-query that can not read INVERTED will return an error

@TCeason had solved a similar compatibility issue like this. What's the correct way for adding a enum variant?

index_type: FromPrimitive::from_i32(p.index_type).ok_or_else(|| Incompatible {
                reason: format!("invalid IndexType: {}", p.index_type),
            })?,

Now index type uses from i32. So we can add a compat test for it? like this:

1. in new query: add inverted index; add AGGREGATING index
2. revert version to main without the inverted index
3. try to query the inverted index; try to query the agg index.

[drmingdrmer]

If there is an incompatible variant, from_pb will return an Error. Is it acceptable? @b41sh

https://github.com/datafuselabs/databend/blob/f9e0e1c7ded521980a1c531cde7dfb7dbd3fd242/src/meta/proto-conv/src/index_from_to_protobuf_impl.rs#L77

[b41sh]

If there is an incompatible variant, from_pb will return an Error. Is it acceptable? @b41sh

Returning an error makes the index unusable, which is unacceptable.

Inverted indexes are for a single table, whereas aggregating indexes are for multiple tables, so it seems better to define them separately. Can we define a new inverted index and put it in TableMeta?

[ariesdevil]

If there is an incompatible variant, from_pb will return an Error. Is it acceptable? @b41sh

Returning an error makes the index unusable, which is unacceptable.

Inverted indexes are for a single table, whereas aggregating indexes are for multiple tables, so it seems better to define them separately. Can we define a new inverted index and put it in TableMeta?↳

Aggregating index also for single table, but one table can have multiple aggregating indexes.

[drmingdrmer]

Can we define a new inverted index and put it in TableMeta?

It would be more clear and neat.

[drmingdrmer]

Reviewed 6 of 72 files at r4, all commit messages.
Reviewable status: 7 of 78 files reviewed, 3 unresolved discussions (waiting on @ariesdevil, @b41sh, and @sundy-li)

---

src/meta/app/src/schema/table.rs line 258 at r4 (raw file):

pub struct TableIndex {
    pub name: String,
    pub columns: Vec<String>,

If a column is renamed, columns becomes invalid.
The column_id should be used instead of String. column_id are unique.

Refer to:
https://github.com/datafuselabs/databend/blob/97e7a31aeaaa8ac9e4e30625b54fc31192081ccd/src/query/expression/src/schema.rs#L156

[drmingdrmer]

Reviewed 1 of 72 files at r4, 3 of 15 files at r5, all commit messages.
Reviewable status: 9 of 78 files reviewed, 4 unresolved discussions (waiting on @ariesdevil, @b41sh, and @sundy-li)

---

src/meta/api/src/schema_api_impl.rs line 3186 at r5 (raw file):

                column_ids: req.column_ids.clone(),
            };
            indexes.insert(req.name.clone(), index);

The column_ids is passed in via the CreateTableIndexReq, the column ids may have changed in the TableMeta fetched at line 3115.

There are two options to address this issue:

• Confirm that the column ids from CreateTableIndexReq present in the TableMeta'
• Specify column by names in the CreateTableIdnexReq, and translate them to column-id in this method.

[drmingdrmer]

Reviewed 30 of 72 files at r4, 3 of 15 files at r5, 3 of 10 files at r6, all commit messages.
Reviewable status: 42 of 78 files reviewed, 2 unresolved discussions (waiting on @ariesdevil, @b41sh, and @sundy-li)