fix(query): onwership/role info serialize to pb

[TCeason]

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

Summary

1. onwership/role info serialize to pb
2. get role/ownership : first try to deserialize data as pb, if failed, deserialize as json. And then, seriazlie it as pb.

• Fixes #[Link the issue here]

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 

[TCeason]

Part 2 of #14380

[TCeason]

Now in this pr, role/ownership will serialize to pb.

In https://github.com/datafuselabs/databend/releases/tag/v1.2.307-nightly support deserialize role/ownership from pb and json.

before v1.2.307 only support deserialize from json.

So if rollback to before v1.2.307, show roles, and then, show grants for role 'role_name', record the grants sql. Drop all user roles in current version.

And then rollback to before v1.2.307. Regenerate the role.

[drmingdrmer]

Nice shot!

Reviewed 1 of 1 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @BohuTANG and @flaneur2020)

[TCeason]

After release, maybe we also need add a not in there? https://github.com/datafuselabs/databend-docs/blob/acc8e8e2a16fa96ca2668db58a286e76bd51caca/docs/guides/10-deploy/12-upgrade/10-compatibility.md#compatibility-between-databend-meta

like compatibility-between-databend-query?

[drmingdrmer]

Reviewed 5 of 8 files at r3.
Reviewable status: 10 of 14 files reviewed, 2 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)

---

src/query/management/src/serde/pb_serde.rs line 105 at r3 (raw file):

    }
    Ok(SeqV::new(seq_value.seq, data))
}

Minimize the indent and return early:

Suggestion:

pub async fn check_and_upgrade_to_pb<'a, T, ErrFn, CtxFn, D>(
    key: String,
    seq_value: &'a SeqV,
    kv_api: Arc<dyn kvapi::KVApi<Error = MetaError> + Send + Sync>,
    err_code_fn: ErrFn,
    context_fn: CtxFn,
) -> std::result::Result<SeqV<T>, ErrorCode>
where
    T: FromToProto + serde::de::DeserializeOwned + 'a + 'static,
    T::PB: databend_common_protos::prost::Message + Default,
    ErrFn: FnOnce(String) -> ErrorCode + std::marker::Copy,
    D: Display,
    CtxFn: FnOnce() -> D + std::marker::Copy,
{
    let deserialize_result =
        deserialize_struct(&seq_value.data, ErrorCode::IllegalUserInfoFormat, || "");

    let err = match deserialize_result {
        Ok(data) => return Ok(SeqV::new(seq_value.seq, data)),
        Err(err) => err,
    };

    log::debug!("deserialize as pb err: {}, rollback to use serde json", err);

    // If there's an error, try to deserialize as JSON.
    let data = serde_json::from_slice::<T>(&seq_value.data)?;

    // If we reached here, it means JSON deserialization was successful but we need to serialize to protobuf format.
    let value = serialize_struct(&data, ErrorCode::IllegalUserInfoFormat, || "")?;
    kv_api
        .upsert_kv(UpsertKVReq::new(
            &key,
            MatchSeq::Exact(seq_value.seq),
            Operation::Update(value),
            None,
        ))
        .await
        .map_err_to_code(err_code_fn, context_fn)?;

    Ok(SeqV::new(seq_value.seq, data))
}

[drmingdrmer]

Reviewed 1 of 8 files at r3, 1 of 1 files at r4, 2 of 3 files at r5, all commit messages.
Reviewable status: 13 of 14 files reviewed, 3 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)

---

src/query/management/tests/it/role.rs line 36 at r5 (raw file):

// and mock!
mock! {

Do not use mock... Just implement a KvApi for test. I mock! introduces non structured code that can not be indexed or refactored...

[drmingdrmer]

Reviewed all commit messages.
Reviewable status: 13 of 14 files reviewed, 3 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)

[TCeason]

All conversation fixed. Please review this pr again. @BohuTANG

[drmingdrmer]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: 12 of 14 files reviewed, 6 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)

---

src/query/management/src/serde/pb_serde.rs line 109 at r9 (raw file):

    }

    Ok(SeqV::new(seq_value.seq, data))

After upsert_kv(), the seq is changed. you should use the new seq to build the return value.

On success, upsert_kv returns a Change<T>, you can find out the new seq in it.
https://github.com/drmingdrmer/databend/blob/991ea29af45e3fea3dd908be7864b6453ad568b0/src/meta/types/src/change.rs#L31

[TCeason]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: 12 of 14 files reviewed, 6 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)

src/query/management/src/serde/pb_serde.rs line 109 at r9 (raw file):

    }

    Ok(SeqV::new(seq_value.seq, data))

After upsert_kv(), the seq is changed. you should use the new seq to build the return value.

On success, upsert_kv returns a Change<T>, you can find out the new seq in it. https://github.com/drmingdrmer/databend/blob/991ea29af45e3fea3dd908be7864b6453ad568b0/src/meta/types/src/change.rs#L31

Push in this commit: 20511a8

[drmingdrmer]

Everything else is great to me:D

Reviewed all commit messages.
Reviewable status: 12 of 14 files reviewed, 7 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)

---

src/query/management/src/serde/pb_serde.rs line 104 at r10 (raw file):

        ))
        .await
        .map_err_to_code(err_code_fn, context_fn)

If you were not going to return this error but just logging it, you do not need to map it.

[TCeason]

Everything else is great to me:D

Reviewed all commit messages.
Reviewable status: 12 of 14 files reviewed, 7 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)

---

src/query/management/src/serde/pb_serde.rs line 104 at r10 (raw file):

        ))
        .await
        .map_err_to_code(err_code_fn, context_fn)

If you were not going to return this error but just logging it, you do not need to map it.

Got but I think maybe better to return a readable err

[TCeason]

Can this PR be merged?

[drmingdrmer]

I do think so.

Reviewed 1 of 2 files at r9, 1 of 1 files at r10.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @BohuTANG, @flaneur2020, and @TCeason)