-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Auto Increment Refactor #886
Conversation
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.
Mostly fine, but you need to fix the information schema thing you broke (or at least add a skipped test for it)
@@ -492,20 +492,23 @@ func rowsAreEqual(ctx *sql.Context, schema sql.Schema, left, right sql.Row) (boo | |||
} | |||
|
|||
// PeekNextAutoIncrementValue peeks at the next AUTO_INCREMENT value | |||
func (t *Table) PeekNextAutoIncrementValue(*sql.Context) (interface{}, error) { |
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.
Problem is this could be a float. If you go this route you need affordances for that
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.
Same comment throughout
} | ||
|
||
return next, nil |
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.
Seems like this is probably enough to handle float columns, but make sure there are tests
@@ -810,7 +809,7 @@ func tablesRowIter(ctx *Context, cat Catalog) (RowIter, error) { | |||
nil, // max_data_length | |||
nil, // max_data_length | |||
nil, // data_free | |||
autoVal, // auto_increment | |||
nil, // auto_increment (always nil) |
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 isn't right, MySQL definitely returns accurate info for this. Why do we need to get rid of 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.
mysql always returns NULL for user space tables
No description provided.