Skip to content

Conversation

@devenbansod
Copy link
Member

Signed-off-by: Deven Bansod devenbansod.bits@gmail.com

@devenbansod
Copy link
Member Author

Hi,
Scrutinizer failed because Code Coverage was not sent to Scrutinizer.

About Travis, sorry but I am unable to understand how my changes caused the test in PMA_FormDisplay_Tpl_Test::testDisplayInput to fail.

Any help would be appreciated.

@ibennetch
Copy link
Member

Scrutinizer failed because Code Coverage was not sent to Scrutinizer.

I'm not sure about this one

About Travis, sorry but I am unable to understand how my changes caused the test in PMA_FormDisplay_Tpl_Test::testDisplayInput to fail.

Right, this is a problem from several commits prior. You can see by clicking on the Details for Travis, then go to the Build history tab, looking down it looks to me like test #13345 (commit d3338ed) is the first failure.

@ibennetch
Copy link
Member

Just an update, I'm having trouble testing this due to a fatal PHP error, but I don't believe the error is related to your commit. I'll try to look again in a few hours.

@lem9
Copy link
Contributor

lem9 commented May 8, 2015

Maybe related, see my comment:
34c28378

@lem9
Copy link
Contributor

lem9 commented May 8, 2015

I suggest rebasing your branch because of a recent fix in master.

@devenbansod
Copy link
Member Author

Thanks @lem9. I did the rebasing. It passes the tests now.

@ibennetch
Copy link
Member

Testing this, I get the error message #1512 - DROP PARTITION can only be used on RANGE/LIST partitions but I don't know much about the use of partitions in this context so it may be that I haven't set something up properly. I'll do further research and see what I can find.

@devenbansod
Copy link
Member Author

Yes. You are right. Thanks for pointing that out.

This two links say :
https://dev.mysql.com/doc/refman/5.1/en/partitioning-management-range-list.html
https://dev.mysql.com/doc/refman/5.1/en/partitioning-management-hash-key.html

Dropping a partition from a table that is partitioned by either RANGE or by LIST can be accomplished using the ALTER TABLE statement with a DROP PARTITION clause

You cannot drop partitions from tables that are partitioned by HASH or KEY in the same way that you can from tables that are partitioned by RANGE or LIST. However, you can merge HASH or KEY partitions using the ALTER TABLE ... COALESCE PARTITION statement.

Will look into it more.

@devenbansod devenbansod force-pushed the rfe1485 branch 2 times, most recently from 7020493 to 854b919 Compare May 9, 2015 04:20
@devenbansod
Copy link
Member Author

Now, a DROP or COALESCE radio button is added according to the type of partition method used.
Workflow when DROP is used is clear that the Partition(s) selected will be dropped while in case of COALESCE, the number of Partitions selected will be coalesced.

@devenbansod devenbansod force-pushed the rfe1485 branch 2 times, most recently from 1d5eba2 to f73a2cd Compare May 9, 2015 08:43
@devenbansod
Copy link
Member Author

PMA_Operations_Test::testGetHtmlForPartitionMaintenance

This test printed output: Not supported query: SELECT PARTITION_METHOD FROM information_schema.PARTITIONS WHERE TABLE_SCHEMA = "db" AND TABLE_NAME = "table"

I am unable to figure out what is its meaning exactly. Any heads-up regarding this ?

@atul516
Copy link
Contributor

atul516 commented May 9, 2015

It appears that you are using a new query via PMA_getHtmlForPartitionMaintenance, so to pass testGetHtmlForPartitionMaintenance, you should add the query to libraries/dbi/DBIDummy.class.php with a valid dummy output for the testsuite to use.

@devenbansod
Copy link
Member Author

Thanks alot @zixtor :)

Have added it as you suggested. Now passing the previously failing test.

@ibennetch
Copy link
Member

Since DROP PARTITION is destructive to data stored in that partition, I think we should also prompt the user (similar to when calling DROP DATABASE or TRUNCATE table) for confirmation. What do you think?

@ibennetch
Copy link
Member

Otherwise this looks good and is close to being ready to merge. The functionality works great in my testing.

@devenbansod
Copy link
Member Author

Yes. Such a check should be done. Because user might not be knowing and he/she might delete it accidentally and would end up losing the data too.

Also, would a simple check on 'submit' like this would be okay ? Or do you suggest using the PMA_confirm as used when clicked on the 'Drop Table' Link ?
I was thinking of this.

        if ( $operation_selected == 'DROP'){
            if (confirm("Are you sure to drop the selected Partition(s)?
                   \nThis will delete the data related to the selected partition(s)")) {
            }
            else {
                event.preventDefault();
            }
        }

@ibennetch
Copy link
Member

I would prefer to use PMA_confirm as in the case of trying to Drop a database, but don't feel particularly strongly about the point; you could probably convince me to change my mind if you have a good reason.

@devenbansod
Copy link
Member Author

Added the Confirmation when DROP partition is selected.

@ibennetch
Copy link
Member

This looks great to me. Nice work. I'll do a bit more testing and merge later tonight, but it looks fine.

…ion Maintenance

Signed-off-by: Deven Bansod <devenbansod.bits@gmail.com>
ibennetch added a commit that referenced this pull request May 11, 2015
@ibennetch ibennetch merged commit 697fd78 into phpmyadmin:master May 11, 2015
@ibennetch
Copy link
Member

Good job.

@devenbansod
Copy link
Member Author

Thanks. :)

@devenbansod devenbansod deleted the rfe1485 branch May 11, 2015 18:41
@devenbansod
Copy link
Member Author

Please see #1681 .

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants