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

ALTER TABLE failing on import when table exists #11407

Closed
llcool opened this Issue Aug 17, 2015 · 27 comments

Comments

Projects
None yet
4 participants
@llcool

llcool commented Aug 17, 2015

-- phpMyAdmin SQL Dump
-- version 4.2.12deb2

-- http://www.phpmyadmin.net

-- Host: localhost
-- Generation Time: Aug 12, 2015 at 10:09 AM
-- Server version: 5.6.25-0ubuntu0.15.04.1
-- PHP Version: 5.6.4-4ubuntu6.2

CREATE TABLE IF NOT EXISTS alert (
alert_id int(11) NOT NULL,
user_id int(11) DEFAULT NULL,
email varchar(80) DEFAULT NULL
) ENGINE=MyISAM AUTO_INCREMENT=29 DEFAULT CHARSET=latin1;
.
.
.
ALTER TABLE alert
ADD PRIMARY KEY (alert_id);

If table structure exists ALTER TABLE will fail with duplicate key and the import exits any other modification will not run.

Why not go back to CREATE TABLE with PRIMARY KEY.

CREATE TABLE IF NOT EXISTS alert (
alert_id int(11) NOT NULL,
user_id int(11) DEFAULT NULL,
email varchar(80) DEFAULT NULL,
PRIMARY KEY (alert_id)
) ENGINE=MyISAM AUTO_INCREMENT=29 DEFAULT CHARSET=latin1;

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 17, 2015

There was a reason to split the statement like this.

@llcool

This comment has been minimized.

llcool commented Aug 17, 2015

lem9, You statement seems incomplete, or am I missing something...

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 17, 2015

See, #6366 and #5703.

But, yes, it is really frustrating when importing a dump into an already existing databases and it fails with similar error. Can we do something about it?

@llcool

This comment has been minimized.

llcool commented Aug 17, 2015

Thank nisargjhaveri, for the info.

Why can't we have ADD PRIMARY KEY IF NOT EXIST

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 17, 2015

@llcool When I have time, I'll search to complete my above comment and clarify the reason why there are two statements. About your suggestion, looking at http://dev.mysql.com/doc/refman/5.6/en/alter-table.html I don't see that it would be possible.

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 17, 2015

Thanks to @nisargjhaveri comment I have read in #6366 that "creating index and foreign keys only after inserting data makes inserting much efficient. "; but the case @llcool was not taken into account.

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 17, 2015

Yes, so I can think of couple of ways in which we can probably improve this.

  1. Actually check if the constraint or key exists or not, when doing alter table. (I don't know how good or bad it would be)
  2. Give an option on export page to choose between whether to separate indexes and table structure or not. (Also quoting, #5703 (comment))
  3. On a slightly different note, when exporting only structure of a table, we don't actually have to separate index creation and rest of table structure, right?
@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 18, 2015

@nisargjhaveri About your 1), it depends: do you want this export to be readable from tools other that phpMyAdmin? If I understand correctly your suggestion, it would be a change of logic at import time from phpMyAdmin. About your 2), it would be a good idea and the option could be "Do you plan to import this into an existing database, where tables already exist". About your 3), the new option suggested in 2) could be shown only when exporting data. Do you wish to work on this?

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 18, 2015

  1. No, I was not talking about change in import logic, I was suggesting something like this question on stackowerflow.

  2. If option is only what you suggested, that can be done by just exporting data. I was thinking about a fallback to standard export where we don't do any alter operations after data for efficiency, and keys are defined in table creating statement itself. Sorry if I misunderstood your comment on this.

  3. I was thinking to set the behaviour in 2) to be default when exporting only structure. No extra options.

Yes, I can work on this.

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 18, 2015

  1. I don't believe that this syntax exists in MySQL (starting a statement with IF NOT EXISTS). 2) Yes there would be an option producing the key definitions in the CREATE TABLE statement, but I'm not sure that this should be the default, as it's not efficient.
@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 18, 2015

Found this: http://dev.mysql.com/doc/refman/5.6/en/if.html but I believe it's only applicable inside routines.

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 18, 2015

Yes @lem9, looks like the IF syntax is indeed applicable only inside routines.

Also, what does IGNORE option do in ALTER TABLE? https://dev.mysql.com/doc/refman/5.5/en/alter-table.html#idm140186428351664

Isn't that we want or is it only for existing conflicting rows and not actual alter table operation.

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 18, 2015

The IGNORE option in ALTER TABLE looks promising, please test it.

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 18, 2015

Sure. I'll test it in a while.

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 18, 2015

IGNORE doesn't work, it still gives "Multiple primary key defined" error.

Another possible workaround might be to create a temporary procedure, call it from script itself and then drop it, as someone suggested here.

The single create table statement looks more human comprehensible, and friendly to some database comparison tools, I don't think that is the solution to the problem reported. It would be nice to have this, though.
But I think, if possible, there at least should be no error if I run an exported script, especially when the table creation already have IF NOT EXISTS.

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 19, 2015

So, offering an option at export time to generate a complete CREATE TABLE statement with indexes (with a warning about reduced efficiency) is the best idea so far.

@madhuracj

This comment has been minimized.

Member

madhuracj commented Aug 20, 2015

I don't know whether a separate option is required. Generally, the use of IF NOT EXISTS is a good indication of whether the user wants to import the SQL file into a database whether tables already exist.

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 20, 2015

I share @madhuracj's view on this.

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 20, 2015

See #11419.

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 20, 2015

@madhuracj But in our export dialog, the IF NOT EXISTS clause of "Add CREATE TABLE statement" is ticked by default, so most exports (at least those done in Quick mode) are likely to generate this clause. Or am I missing something?

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 20, 2015

Yes, but we can probably make it unchecked by default, and also add efficiency warning.
The main reason to do this is, even if IF NOT EXISTS clause is there, it is useless, because keys are too common, and the import will fail there. So, if it is anyway going to fail, so why not on table creation. And if anyone wants IF NOT EXISTS specifically, they can enable it.

@madhuracj

This comment has been minimized.

Member

madhuracj commented Aug 21, 2015

I also think it make sense to change the default of IF NOT EXISTS

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 21, 2015

Ok. Nisarg can you update your commit as in your suggestion?

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 21, 2015

Okay. I'll make it unchecked by default.
Also, we want to add efficiency warning along with IF NOT EXISTS, right?

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 21, 2015

Yes, an efficiency warning next to IF NOT EXISTS is important.

@lem9

This comment has been minimized.

Contributor

lem9 commented Aug 21, 2015

By the way, these changes would go to 4.5, right?

@lem9 lem9 self-assigned this Aug 21, 2015

@nisargjhaveri

This comment has been minimized.

Contributor

nisargjhaveri commented Aug 21, 2015

@lem9 lem9 added this to the 4.5.0 milestone Aug 22, 2015

@lem9 lem9 closed this Aug 22, 2015

@lem9 lem9 changed the title from phpMyAdmin SQL Dump - Export/Import (CREATE TABLE IF NOT EXISTS) to ALTER TABLE failing on import when table exists Aug 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment