Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove QueryRequest from GreptimeRequest #3799

Open
MichaelScofield opened this issue Apr 25, 2024 · 3 comments
Open

Remove QueryRequest from GreptimeRequest #3799

MichaelScofield opened this issue Apr 25, 2024 · 3 comments
Labels
good first issue Good for newcomers

Comments

@MichaelScofield
Copy link
Collaborator

What type of enhancement is this?

Refactor

What does the enhancement do?

We have the QueryRequest in GreptimeRequest:

message GreptimeRequest {
  RequestHeader header = 1;
  oneof request {
    InsertRequests inserts = 2;
    QueryRequest query = 3; <-- here
    DdlRequest ddl = 4;
    DeleteRequests deletes = 5;
    RowInsertRequests row_inserts = 6;
    RowDeleteRequests row_deletes = 7;
  }
}

However, now the GreptimeRequest is not supposed to be used for query. So the QueryRequest in it (but not the QueryRequest itself!) can be deleted, along with some codes.

Implementation challenges

No response

@tisonkun
Copy link
Contributor

IIUC we should remove the definition in https://github.com/GreptimeTeam/greptime-proto first?

@MichaelScofield
Copy link
Collaborator Author

@tisonkun right, but normally we first make the changes to proto at our own fork, then change the db codes, and finally merge the proto.

@tisonkun
Copy link
Contributor

I found same of the dependencies to QueryRequest is from our inner client Database, that is related to #3798.

So this issue can be blocked from #3798.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants