-
Notifications
You must be signed in to change notification settings - Fork 45
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
Address PK issues for show create table queries #127
Address PK issues for show create table queries #127
Conversation
} | ||
|
||
if ( '' !== $column->dflt_value ) { | ||
$definition[] = 'DEFAULT ' . $column->dflt_value; // quotes? |
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.
Could we please expand this comment, it's unclear what it means.
// If the PK columns are the same as the unique key columns, skip the key. | ||
// This is because the PK is already unique in MySQL. |
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.
Nice, these comments help a lot.
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.
@jeroenpf the PR looks good to me. Thanks for working on this.
I just left one question about tests after we answer that I'm happy to merge the PR.
Thanks @bgrgicak for the review. I've addressed your latest comment so we should be good to go. |
Fixes: #123
In this PR i propose a refactor of the handling of
show create table
queries.The main issue was that it is possible for a primary key to be composite (more than one column). The existing implementation only supports a single PK which causes the
CREATE TABLE
statement to be invalid. An example of such a table is thewp_term_relationships
found in WordPress:SQLite also creates a unique key for any primary key (single or composite) which is something that is not necessary for MySQL and should be skipped from the final create statement.
In this PR
The final query output for
show create wp_term_relationships
looks as follows:Testing
$wpdb->query('show create wp_term_relationships')
- The result should be a valid MySQL compatible query as shown above.