-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Error happened when open a view in DBeaver #4778
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
Conversation
|
It is best to talk about the cause of the problem and the idea of PR change. |
| // table(view)'s comment | ||
| protected String comment = ""; | ||
|
|
||
| protected String ddlSql = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment for this new field
| if (NULL == str_slot->ptr) { | ||
| return Status::InternalError("Allocate memcpy failed."); | ||
| } | ||
| memcpy(str_slot->ptr, definer.c_str(), str_slot->len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
length always 6 and definer always "root@%" ? why ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is column for table views in mysql, palo has no this information, we can only set a default one
| .column("CHECK_OPTION", ScalarType.createVarchar(8)) | ||
| .column("IS_UPDATABLE", ScalarType.createVarchar(3)) | ||
| .column("DEFINER", ScalarType.createVarchar(77)) | ||
| .column("SECURITY_TYPE", ScalarType.createVarchar(7)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this column length is 77, you should add a comment, this column in mysql is 288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you get another version of mysql. the sql I got is like this.
| .column("DEFINER", ScalarType.createVarchar(77)) | ||
| .column("SECURITY_TYPE", ScalarType.createVarchar(7)) | ||
| .column("CHARACTER_SET_CLIENT", ScalarType.createVarchar(32)) | ||
| .column("COLLATION_CONNECTION", ScalarType.createVarchar(32)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the sql I got is like this.
| default: | ||
| tables = db.getTables(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | |
| List<Table> tables = db.getTables(); | |
| if (params.isSetType() && "VIEW".equals(params.getType())) { | |
| tables = db. getViews(); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If type is view, this will call getTables() which is not required. When the count of tables in this db is too many, this will take more time.
yangzhg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
morningman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Proposed changes
Fix bug #4777:Error happened when open a view in DBeaver
Types of changes
What types of changes does your code introduce to Doris?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
Fix bug #4777