[go: nahoru, domu]
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change default database name in Operations > Copy database to #13099

Closed
yevgenkrylov opened this issue Mar 18, 2017 · 18 comments
Closed

Change default database name in Operations > Copy database to #13099

yevgenkrylov opened this issue Mar 18, 2017 · 18 comments
Assignees
@nijel
Labels
Bug A problem or regression with an existing feature
Milestone
4.8.0

Comments

@yevgenkrylov
Copy link

Steps to reproduce

  1. Go to "Databases" tab, select your database
  2. Go to "Operations" tab
  3. Click "Go" under "Copy database to" section (without changing pre-populated database name)

Expected behaviour

  • get a copy of your database with a different name (append something unique to the database name by default) OR
  • get a warning, that user is going to destroy the database by copying it to itself OR
  • do not pre-populate database name in Copy to section OR
  • your option...

Actual behaviour

Get data in your database corrupted.
I really do not understand what might be a real use case for the default behavior of pma, when it populates current database name...

Server configuration

Operating system: CentOS Linux release 7.3.1611 (Core)

Web server: Apache 2.4.25

Database: 10.1.21-MariaDB - MariaDB Server

PHP version: 5.6.30

phpMyAdmin version: 4.6.6

Client configuration

Browser: Chrome

Operating system: Windows 10
@Achilles-96
Copy link
Contributor

It does throw an error.
image

@yevgenkrylov
Copy link
Author

I forgot to metion one step: tick "drop table/view" box...

Achilles-96 added a commit to Achilles-96/phpmyadmin that referenced this issue Mar 19, 2017
@Achilles-96
Adds non-conflicting default new database name phpmyadmin#13099
c4c4c8b 
Signed-off-by: Raghuram Vadapalli<raghuram.vadapalli@research.iiit.ac.in>
Achilles-96 added a commit to Achilles-96/phpmyadmin that referenced this issue Mar 19, 2017
@Achilles-96
Adds non-conflicting default new database name phpmyadmin#13099
7bad0d6 
Signed-off-by: Raghuram Vadapalli<raghuram.vadapalli@research.iiit.ac.in>
@SaturnTeam SaturnTeam mentioned this issue Mar 19, 2017
Closed
@SaturnTeam
Copy link

I'm fixing it. Just need to write test for it

@Achilles-96
Copy link
Contributor
Achilles-96 commented Mar 19, 2017 •
edited
Loading

@thesaturn , I just made sure that default name is not already existing. Try to look into why data is getting corrupted if the db name is same and fix it. I feel that is the major issue here :) .

@ibennetch
Copy link
Member

Related to #12233 which was about the "Rename" dialog, not copy.

@ibennetch
Copy link
Member

We should definitely catch cases when the user tries this and provide a reasonable error message, but I think we also should remove the default suggestion from the copy and rename fields.

@Achilles-96
Copy link
Contributor
Achilles-96 commented Mar 19, 2017 •
edited
Loading

@ibennetch , Any specific reason why you want defaults to be removed? I feel they are good to enable user to create a copy of db within a single click (typical use of any default) .

@SaturnTeam
Copy link

So I tried to fix it. I wrote a message and check if new name is the same as current. But I don't know how to write selenium test for that

@nijel nijel added the Bug A problem or regression with an existing feature label Mar 20, 2017
Achilles-96 added a commit to Achilles-96/phpmyadmin that referenced this issue Mar 21, 2017
@Achilles-96
Adds non-conflicting default new database name phpmyadmin#13099
7a703cb 
Signed-off-by: Raghuram Vadapalli<raghuram.vadapalli@research.iiit.ac.in>
Achilles-96 added a commit to Achilles-96/phpmyadmin that referenced this issue Mar 21, 2017
@Achilles-96
Removes default new database name phpmyadmin#13099
4ca8cfb 
Signed-off-by: Raghuram Vadapalli<raghuram.vadapalli@research.iiit.ac.in>
nijel pushed a commit that referenced this issue Mar 21, 2017
@Achilles-96 @nijel
Removes default new database name
553d1ff 
Issue #13099

Signed-off-by: Raghuram Vadapalli<raghuram.vadapalli@research.iiit.ac.in>
Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel nijel added this to the 4.7.0 milestone Mar 21, 2017
@nijel nijel self-assigned this Mar 21, 2017
@nijel nijel modified the milestones: 4.8.0, 4.7.0 Mar 21, 2017
@nijel
Copy link
Contributor
nijel commented Mar 21, 2017

The default value has been fixed in 553d1ff, the error check on same db name is pending in #13104.

@OlafvdSpek
Copy link
OlafvdSpek commented Mar 23, 2017 •
edited
Loading

The DROP TABLES checkbox should be removed as well.. dropping tables should be a separate operation.

Perhaps an additional check to ensure the destination is empty?

@yevgenkrylov
Copy link
Author

The DROP TABLES could be useful - for example, for making a fresh copy of production environment for test and so on.

@OlafvdSpek
Copy link

One can easily drop all tables before importing the DB.
And if the old DB contains more tables then the new DB some old stuff is left behind if you don't start from a clean slate.

@yevgenkrylov
Copy link
Author

Well... yes, there are workarounds, but if this option exists on command line I do not see a reason not to implement it in GUI as well (where it is not selected by default).
I mean, there might be lots of people using it for reasons we do not know...
This issue is about changing dangerous defaults (same database name), not about making phpmyadmin "foolproof".

@OlafvdSpek
Copy link
OlafvdSpek commented Mar 23, 2017 •
edited
Loading

Dropping tables is dangerous..

but if this option exists on command line

Does pMA have a command line? :p

@yevgenkrylov
Copy link
Author

mysqldump --add-drop-table

@OlafvdSpek
Copy link

One doesn't write --add-drop-table by accident, clicking a checkbox however..

--add-drop-table is on by default though, which I think is a mistake for the same reason.

@yevgenkrylov
Copy link
Author
yevgenkrylov commented Mar 23, 2017 •
edited
Loading

is a mistake for the same reason

you didn't mention any reasoning in your original suggestion... could you please elaborate?

@OlafvdSpek
Copy link
  1. Dropping tables isn't required to be part of creating tables / importing data, so it's optional.
  2. It complicates the copy / export operation.
  3. It's too easy to destroy data.

@nijel nijel closed this as completed in dbdd7bc Apr 12, 2017
@madhuracj madhuracj mentioned this issue Aug 29, 2017
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@nijel nijel

Labels
Bug A problem or regression with an existing feature
Projects
None yet
Milestone
4.8.0
Development

No branches or pull requests
6 participants
@nijel @ibennetch @OlafvdSpek @SaturnTeam @Achilles-96 @yevgenkrylov