-
Notifications
You must be signed in to change notification settings - Fork 85
Unify --filesize
and --statement-size
definition with mydumper's
#142
Conversation
v4/export/ir.go
Outdated
HasNext(currentStatementSize, currentFileSize uint64) bool | ||
HasNextSQLRowIter(currentFileSize uint64) bool | ||
AdjustFileRowIterSize(currentStatementSize, currentFileSize uint64) |
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 must these passed as arguments into the iterator?
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.
Because currently the written size is saved in writer
but the FilesizeLimit
and StatementsizeLimit
is saved in file row iterator
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.
gotta say this solution doesn't look good to me. the SQLRowIter
should only care about pumping out *sql.Row
s, it should be the CSVWriter/SQLWriter's responsibility to know when to add the statement separator and when to create a new file.
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 see. I will take thought on how to refine this.
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.
revised, PTAL again
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.
rest LGTM
v4/export/writer_util.go
Outdated
if b.fileSizeLimit != UnspecifiedSize && b.currentFileSize >= b.fileSizeLimit { | ||
return true | ||
} | ||
return false |
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 b.fileSizeLimit != UnspecifiedSize && b.currentFileSize >= b.fileSizeLimit { | |
return true | |
} | |
return false | |
return b.fileSizeLimit != UnspecifiedSize && b.currentFileSize >= b.fileSizeLimit |
v4/export/writer_util.go
Outdated
if b.fileSizeLimit != UnspecifiedSize && b.currentFileSize >= b.fileSizeLimit { | ||
return true | ||
} | ||
if b.statementSizeLimit != UnspecifiedSize && b.currentStatementSize >= b.statementSizeLimit { | ||
return true | ||
} | ||
return false |
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.
ditto
…ingcap#142) * reduce bound address times to save more time * tmp * unify file size * fix ut * fix bug that escapeBackSlash not used for rows arguments * tmp * move filesize and statementsize argument to writepipe * fix format * address comment
…ingcap#142) * reduce bound address times to save more time * tmp * unify file size * fix ut * fix bug that escapeBackSlash not used for rows arguments * tmp * move filesize and statementsize argument to writepipe * fix format * address comment
…ingcap#142) * reduce bound address times to save more time * tmp * unify file size * fix ut * fix bug that escapeBackSlash not used for rows arguments * tmp * move filesize and statementsize argument to writepipe * fix format * address comment
…ingcap#142) * reduce bound address times to save more time * tmp * unify file size * fix ut * fix bug that escapeBackSlash not used for rows arguments * tmp * move filesize and statementsize argument to writepipe * fix format * address comment
…ingcap/dumpling#142) * reduce bound address times to save more time * tmp * unify file size * fix ut * fix bug that escapeBackSlash not used for rows arguments * tmp * move filesize and statementsize argument to writepipe * fix format * address comment
What problem does this PR solve?
fix #127
What is changed and how it works?
Don't only count data for
statement-size
andfilesize
options.Check List
Tests
Related changes
Release note
unify
--filesize
and--statement-size
definition with mydumper's