This repository has been archived by the owner on Nov 24, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
*: support generated column in loader and binlog replication #42
*: support generated column in loader and binlog replication #42
Changes from 7 commits
0c1c957
c18c5ab
cc6b6b5
a54efe0
1ee6ab7
bf5dba9
13182d9
9443dfe
1641de0
0af60d6
5dffffa
1841376
048078f
d3572b7
c42b151
091443c
7a027b3
cb7dd1e
5464994
d731ce1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
rules
only contains one rule,butexpected
contains two caseThere 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.
I would use restore to implement it later
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.
update example at L365
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.
must
extra
be uppercase? should we use== strings.TrimSpace(xxx)
rather thanstrings.Contains
?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.
according to MySQL doc,
VIRTUAL GENERATED
andSTORED GENERATED
inExtra
field are always uppercaseThere 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.
strings.Contains
is used because there may exist other data inExtra
?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.
I'm afraid some comment options are in it
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.
https://dev.mysql.com/doc/refman/5.7/en/show-columns.html
In MySQL doc, there are only four Extra fields:
auto_increment
,on update CURRENT_TIMESTAMP
,VIRTUAL GENERATED
orVIRTUAL STORED
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.
is
TestResolveGeneratedColumnDDL
better?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.
would generate column be one of unique columns?
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.
only secondary index is allowed on generated columns.