Merge lp:~sergei.glushchenko/percona-xtrabackup/bug711166_part-1.6 into lp:percona-xtrabackup/1.6

[Sergei Glushchenko]

Bug 711166
Implement full support for backups of partitioned tables.

Fixed support for partitioned tables (both innobackupex and xtrabackup.c)
when --tables-file option specified. This also fixes support for
partitioned tables when --databases option specified. Two testcases
added. First one for MyISAM partitioned table with --tables-file option
and DATA DIRECTORY/INDEX DIRECTORY specified, second one for InnoDB
partitioned table with --tables-file option.

[Sergei Glushchenko (sergei.glushchenko)]

Jenkins build:
http://jenkins.percona.com/view/Percona%20Xtrabackup/job/percona-xtrabackup-1.6-param/107/

[Alexey Kopytov (akopytov)]

Sergei,

Partial backups can also be created with the --include option of innobackupex (or its xtrabackup equivalent, --tables). That does not seem to be covered by the fix.

I've amended the bug report so we take that into account in the fix.

Some minor comments:

- innobackupex should be used in tests rather than IB_BIN directly

- please use --copy-back to restored when innobackupex is used to create a backup, so we also test that functionality

- you can use the checksum_table() function from common.sh

- xtrabackup.c (mostly) follows the InnoDB coding style. so the closin */ in comments should be on the last comment line, rather than on a separate one.

- typo ("Cheking")

- typo ("parttition")

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

Alexei,

> Sergei,
>
> Partial backups can also be created with the --include option of innobackupex
> (or its xtrabackup equivalent, --tables). That does not seem to be covered by
> the fix.
>

--include option passed directly to --tables option of xtrabackup.
Each database.table value passed to --tables option considered as regular
expression. So if we pass --tables=test.test, all partitions of database test
would be backed up because of test.test matches all files test/test#P#*
I think it's a good behavior.

Thanks for your comments! I'll do my best to fix tescases.

[Alexey Kopytov (akopytov)]

Sergei,

On 01.02.12 21:20, Sergei Glushchenko wrote:
> Alexei,
>
>> Sergei,
>>
>> Partial backups can also be created with the --include option of innobackupex
>> (or its xtrabackup equivalent, --tables). That does not seem to be covered by
>> the fix.
>>
>
> --include option passed directly to --tables option of xtrabackup.

Not always. It is handled in innobackupex for streaming backups.

> Each database.table value passed to --tables option considered as regular
> expression. So if we pass --tables=test.test, all partitions of database test
> would be backed up because of test.test matches all files test/test#P#*
> I think it's a good behavior.
>

Consider a case when I have tables 'test' and 'test_table'. So to be
able to backup only 'test', but not 'test_table', I have to use
--tables='test$'. But that won't work if 'test' is partitioned, right?

[Sergei Glushchenko (sergei.glushchenko)]

Bug 711166
Partitioned tables are not correctly handled by the --databases, --include,
--tables-file options of innobackupex, and by the --tables and
--tables-file options of xtrabackup.
Solution is to remove partition suffix (#P#...) before doing filtering.
Testcases cover variants of using filtering options with MyISAM and
InnoDB tables (to test both innobackupex and xtrabackup) with either stream
mode turned on and turned off.

Since last MP request:
 added --include option handling
 added more testcases

Jenkins build:
lp:~sergei.glushchenko/percona-xtrabackup/bug711166_part-1.6

[Sergei Glushchenko (sergei.glushchenko)]

Jenkins build:
http://jenkins.percona.com/view/Percona%20Xtrabackup/job/percona-xtrabackup-1.6-param/110/

[Alexey Kopytov (akopytov)]

Sergei,

   - I think you can avoid some code duplication in test cases by
     putting the table definition to a common file in the inc/ directory
     and using the default_storage_engine server variable.
   - you can also reduce the test sizes considerably by not messing with
     information_schema and performance_schema. I_S is actually not a
     storage engine, so it doesn't have any files in the data dir. And
     you could just drop the test database before shutting down the
     server and restoring, rather than just remove the entire data dir,
     so you don't have to care about performance_schema and mysql DBs. I
     think that would make test cases much smaller and easier to read,
     but without any impact on test coverage.
   - you can also make tests faster by not inserting 500 rows into the
     table. Inserting that many rows one-by-one by invoking the client
     each time actually take significant time. I think the following
     would be sufficient for your purposes:
       INSERT INTO test VALUES (1), (101), (201), (301), (401);
   - Unlike the core server, InnoDB code encloses "if" blocks in braces
     even if the block consists of only 1 statement.
   - I think changes in xtrabackup.c have to much duplication and
     unnecessary work. The whole point of those transformations is to
     check whether a specific table should be skipped. Please take a
     look at my changes to that code here
     http://bazaar.launchpad.net/~akopytov/percona-xtrabackup/compact-backups/view/head:/src/xtrabackup.c#L1550
     Perhaps it even makes sense to backport check_if_skip_table() from
     compact backups branch to 1.6/trunk and base your fix on that
     function?

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

Alexey,

> - I think you can avoid some code duplication in test cases by
> putting the table definition to a common file in the inc/ directory
> and using the default_storage_engine server variable.

yes. You're right about that.

> - you can also reduce the test sizes considerably by not messing with
> information_schema and performance_schema. I_S is actually not a
> storage engine, so it doesn't have any files in the data dir. And
> you could just drop the test database before shutting down the
> server and restoring, rather than just remove the entire data dir,
> so you don't have to care about performance_schema and mysql DBs. I
> think that would make test cases much smaller and easier to read,
> but without any impact on test coverage.

We can't just drop database `test` and keep other databases in their places.
copy-back function expects that data directory is empty when we are
restoring backup.

> - you can also make tests faster by not inserting 500 rows into the
> table. Inserting that many rows one-by-one by invoking the client
> each time actually take significant time. I think the following
> would be sufficient for your purposes:
> INSERT INTO test VALUES (1), (101), (201), (301), (401);

OK. I'll follow your advice.

> - Unlike the core server, InnoDB code encloses "if" blocks in braces
> even if the block consists of only 1 statement.

Thanks. I should print InnoDB code guidelines somewhere and put it somewhere over my eyes.

> - I think changes in xtrabackup.c have to much duplication and
> unnecessary work. The whole point of those transformations is to
> check whether a specific table should be skipped. Please take a
> look at my changes to that code here
> http://bazaar.launchpad.net/~akopytov/percona-xtrabackup/compact-
> backups/view/head:/src/xtrabackup.c#L1550
> Perhaps it even makes sense to backport check_if_skip_table() from
> compact backups branch to 1.6/trunk and base your fix on that
> function?

As I can see check_if_skip_table does pretty much the same thing, but does
in-place patching instead of copying. I tried to avoid in-place patching,
because of it is not good from my point of view to modify data which is
not belongs to us. You should look at xtrabackup_stats_func in your
branch. It contains pretty much the same code as check_if_skip_table.

I'm surely can write something similar to check_if_skip_table, but avoiding
in-place patching. What do you think about that?

[Alexey Kopytov (akopytov)]

Sergei,

On 17.02.12 14:05, Sergei Glushchenko wrote:
>> - you can also reduce the test sizes considerably by not messing with
>> information_schema and performance_schema. I_S is actually not a
>> storage engine, so it doesn't have any files in the data dir. And
>> you could just drop the test database before shutting down the
>> server and restoring, rather than just remove the entire data dir,
>> so you don't have to care about performance_schema and mysql DBs. I
>> think that would make test cases much smaller and easier to read,
>> but without any impact on test coverage.
>
> We can't just drop database `test` and keep other databases in their places.
> copy-back function expects that data directory is empty when we are
> restoring backup.
>

Right, which means --copy-back is useless for partial backups. It either
creates an unusable datadir, if it was empty originally, or just fails,
if it wasn't empty.

Which in turn means users are expected to restore manually from a
partial backups. And so we should do the same in the test suite?

>
>> - Unlike the core server, InnoDB code encloses "if" blocks in braces
>> even if the block consists of only 1 statement.
>
> Thanks. I should print InnoDB code guidelines somewhere and put it somewhere over my eyes.
>

The problem is that there's no such thing as InnoDB code guidelines. If
you find them, let me know! :)

>> - I think changes in xtrabackup.c have to much duplication and
>> unnecessary work. The whole point of those transformations is to
>> check whether a specific table should be skipped. Please take a
>> look at my changes to that code here
>> http://bazaar.launchpad.net/~akopytov/percona-xtrabackup/compact-
>> backups/view/head:/src/xtrabackup.c#L1550
>> Perhaps it even makes sense to backport check_if_skip_table() from
>> compact backups branch to 1.6/trunk and base your fix on that
>> function?
>
> As I can see check_if_skip_table does pretty much the same thing, but does
> in-place patching instead of copying. I tried to avoid in-place patching,
> because of it is not good from my point of view to modify data which is
> not belongs to us. You should look at xtrabackup_stats_func in your
> branch. It contains pretty much the same code as check_if_skip_table.
>
> I'm surely can write something similar to check_if_skip_table, but avoiding
> in-place patching. What do you think about that?
>

Sure, my point was that encapsulating the entire logic to decide whether
a table should be skipped or not is better than implementing some
auxiliary functions and passing the buffers around.

In-place modifications are certainly not nice, I just didn't have time
to fix that too, as I was after something completely different, so I
left that work for better times. Such as now :)

But thanks for pointing out the same code is in xtrabackup_stats_func().
I didn't notice it, because the main object of refactoring was
xtrabackup_copy_datafile().

OTOH I think malloc()s can be avoided and replaced with a
stack-allocated bufer.

[Sergei Glushchenko (sergei.glushchenko)]

Tescases were reduced and improved a little. It allowed to find mistake in testcase and implementation. (tar command in stream mode didn't relsolve symbolic likns). check_if_skip_table was implemented instead of auxiliary functions.

Jenkins:
http://jenkins.percona.com/view/Percona%20Xtrabackup/job/percona-xtrabackup-1.6-param/119/

[Sergei Glushchenko (sergei.glushchenko)]

Updated with current 1.6 trunk.
lp:~sergei.glushchenko/percona-xtrabackup/bug711166_part-1.6

[Sergei Glushchenko (sergei.glushchenko)]

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-1.6-param/147/

[Alexey Kopytov (akopytov)]

Sergei,

As discussed previously, we will not be fixing this bug in 1.6. Please create a 2.0 MP.

Unmerged revisions

348. By Sergei Glushchenko on 2012-09-18

Partitioned tables are not correctly handled by the --databases, --include,
--tables-file options of innobackupex, and by the --tables and
--tables-file options of xtrabackup.
Solution is to remove partition suffix (#P#...) before doing filtering.
Testcases cover variants of using filtering options with MyISAM and
InnoDB tables (to test both innobackupex and xtrabackup) with either stream
mode turned on and turned off.

=== modified file 'innobackupex'
--- innobackupex	2012-06-08 18:46:19 +0000
+++ innobackupex	2012-09-18 13:09:33 +0000
@@ -975,11 +975,9 @@
                         if($option_include) {
                             my $table_name;
 
-                            $table_name = substr($file, rindex($file, '/'));
-                            $table_name = substr($table_name, 1, rindex($table_name, '.') - 1);
-                            $table_name = $subdir . "." . $table_name;
+                            $table_name = get_table_name($file);
 
-                            if (!($table_name =~ /$option_include/)) {
+                            if (!("$subdir.$table_name" =~ /$option_include/)) {
                                 print STDERR "'$file' is skipped.\n";
                                 next;
                             }
@@ -1984,7 +1982,8 @@
             next unless check_if_required($database, $file);
 
 	    if($option_include) {
-	      if (!("$database.$file" =~ /$option_include/)) {
+	      my $table_name = get_table_name($file);
+	      if (!("$database.$table_name" =~ /$option_include/)) {
 	        print STDERR "$database.$file is skipped because it does not match $option_include.\n";
 	        next;
 	      }
@@ -2013,7 +2012,7 @@
                 my $ret = 0;
                 my $file_name = substr($file, rindex($file, '/') + 1);
                 $file_name=~s/([\$\\\" ])/\\$1/g;
-                $ret = system("cd $source_dir; tar cf - $database/$file_name") >> 8;
+                $ret = system("cd $source_dir; tar chf - $database/$file_name") >> 8;
                 if ($ret == 1) {
                     print STDERR "$prefix If you use GNU tar, this warning can be ignored.\n";
                 # Check for non-zero exit code
@@ -2303,6 +2302,27 @@
     return ${$group_hash_ref}{$option_name};
 }
 
+# get_table_name subroutine returns table name of specified file.
+#    Parameters:
+#       $_[0]        table path
+#    Return value:
+#       1 table name
+#
+sub get_table_name {
+   my $table_path = shift;
+   my $filename;
+   my $table;
+
+   # get the last component in the table pathname 
+   $filename = (reverse(split(/\//, $table_path)))[0];
+   # get name of the table by removing file suffix
+   $table = (split(/\./, $filename))[0];
+   # and partition suffix
+   $table = (split('#P#', $table))[0];
+
+   return $table;
+}
+
 # check_if_required subroutine returns 1 if the specified database and
 # table needs to be backed up.
 #    Parameters:
@@ -2315,7 +2335,6 @@
    my ( $db, $table_path ) = @_;
    my $db_count  = scalar keys %databases_list;
    my $tbl_count = scalar keys %table_list;
-   my $filename;
    my $table;
 
    if ( $db_count == 0 && $tbl_count == 0 ) {
@@ -2325,10 +2344,7 @@
    }
    else {
       if ( $table_path ) {
-         # get the last component in the table pathname 
-         $filename = (reverse(split(/\//, $table_path)))[0];
-         # get name of the table by removing file suffix
-         $table = (split(/\./, $filename))[0];
+         $table = get_table_name($table_path);
       }
    }
 
 
=== added file 'test/inc/ib_part.sh'
--- test/inc/ib_part.sh	1970-01-01 00:00:00 +0000
+++ test/inc/ib_part.sh	2012-09-18 13:09:33 +0000
@@ -0,0 +1,76 @@
+
+function ib_part_schema()
+{
+	topdir=$1
+	engine=$2
+
+	cat <<EOF
+CREATE TABLE test (
+  a int(11) DEFAULT NULL
+) ENGINE=$engine DEFAULT CHARSET=latin1
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (100) ENGINE = $engine,
+ PARTITION P1 VALUES LESS THAN (200) ENGINE = $engine,
+ PARTITION p2 VALUES LESS THAN (300)
+   DATA DIRECTORY = '$topdir/ext' INDEX DIRECTORY = '$topdir/ext'
+   ENGINE = $engine,
+ PARTITION p3 VALUES LESS THAN (400)
+   DATA DIRECTORY = '$topdir/ext' INDEX DIRECTORY = '$topdir/ext'
+   ENGINE = $engine,
+ PARTITION p4 VALUES LESS THAN MAXVALUE ENGINE = $engine);
+EOF
+}
+
+function ib_part_data()
+{
+	echo 'INSERT INTO test VALUES (1), (101), (201), (301), (401);';
+}
+
+function ib_part_init()
+{
+	topdir=$1
+	engine=$2
+
+	if [ -d $topdir/ext ] ; then
+		rm -rf $topdir/ext
+	fi
+	mkdir -p $topdir/ext
+
+	ib_part_schema $topdir $engine | run_cmd $MYSQL $MYSQL_ARGS test
+	ib_part_data $topdir $engine | run_cmd $MYSQL $MYSQL_ARGS test
+}
+
+function ib_part_restore()
+{
+	topdir=$1
+	mysql_datadir=$2
+
+	# Remove database
+	rm -rf $mysql_datadir/test/*
+	rm -rf $topdir/ext/*
+	vlog "Original database removed"
+
+	# Restore database from backup
+	cp -rv $topdir/backup/test/* $mysql_datadir/test
+	vlog "database restored from backup"
+
+}
+
+function ib_part_assert_checksum()
+{
+	checksum_a=$1
+
+	vlog "Checking checksums"
+	checksum_b=`checksum_table test test`
+
+	vlog "Checksums are $checksum_a and $checksum_b"
+
+	if [ "$checksum_a" != "$checksum_b" ]
+	then 
+		vlog "Checksums are not equal"
+		exit -1
+	fi
+
+	vlog "Checksums are OK"
+
+}
 
=== added file 'test/t/ib_part_databases.sh'
--- test/t/ib_part_databases.sh	1970-01-01 00:00:00 +0000
+++ test/t/ib_part_databases.sh	2012-09-18 13:09:33 +0000
@@ -0,0 +1,42 @@
+########################################################################
+# Bug #711166: Partitioned tables are not correctly handled by the
+#              --databases and --tables-file options of innobackupex,
+#              and by the --tables option of xtrabackup.
+#              Testcase covers using --databases option with MyISAM
+#              database
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server
+
+if ! ${MYSQL} ${MYSQL_ARGS} -e "SHOW VARIABLES LIKE '%partition%';" | grep YES ; then
+	echo "Requires partition support" > $SKIPPED_REASON
+	exit $SKIPPED_EXIT_CODE
+fi
+
+# Create MyISAM partitioned table with some partitions in
+# different location
+ib_part_init $topdir MyISAM
+
+# Saving the checksum of original table
+checksum_a=`checksum_table test test`
+
+# Take a backup
+cat >$topdir/databases_file <<EOF
+test.test
+EOF
+innobackupex --no-timestamp --databases=$topdir/databases_file $topdir/backup
+innobackupex --apply-log $topdir/backup
+vlog "Backup taken"
+
+stop_server
+
+# Restore partial backup
+ib_part_restore $topdir $mysql_datadir
+
+start_server
+
+# compare checksum
+ib_part_assert_checksum $checksum_a
 
=== added file 'test/t/ib_part_include.sh'
--- test/t/ib_part_include.sh	1970-01-01 00:00:00 +0000
+++ test/t/ib_part_include.sh	2012-09-18 13:09:33 +0000
@@ -0,0 +1,51 @@
+########################################################################
+# Bug #711166: Partitioned tables are not correctly handled by the
+#              --databases and --tables-file options of innobackupex,
+#              and by the --tables option of xtrabackup.
+#              Testcase covers using --include option with InnoDB
+#              database
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server --innodb_file_per_table
+
+if ! ${MYSQL} ${MYSQL_ARGS} -e "SHOW VARIABLES LIKE '%partition%';" | grep YES ; then
+	echo "Requires partition support" > $SKIPPED_REASON
+	exit $SKIPPED_EXIT_CODE
+fi
+
+# Create InnoDB partitioned table
+ib_part_init $topdir InnoDB
+
+# Saving the checksum of original table
+checksum_a=`checksum_table test test`
+
+# Take a backup
+# Only backup of test.test table will be taken
+cat >$topdir/tables <<EOF
+test.test
+EOF
+innobackupex --no-timestamp --include='test.test$' $topdir/backup
+innobackupex --apply-log $topdir/backup
+vlog "Backup taken"
+
+# also test xtrabackup --stats work with --tables-file
+COUNT=`xtrabackup --stats --tables='test.test$' --datadir=$topdir/backup 2>&1 \
+       | grep table: | awk '{print $2}' | sort -u | wc -l`
+
+if [ $COUNT != 5 ] ; then
+	xtrabackup --stats --tables='test.test$' --datadir=$topdir/backup
+	vlog "xtrabackup --stats does not work. count = $COUNT"
+	exit -1
+fi
+
+stop_server
+
+# Restore partial backup
+ib_part_restore $topdir $mysql_datadir
+
+start_server
+
+ib_part_assert_checksum $checksum_a
 
=== added file 'test/t/ib_part_include_stream.sh'
--- test/t/ib_part_include_stream.sh	1970-01-01 00:00:00 +0000
+++ test/t/ib_part_include_stream.sh	2012-09-18 13:09:33 +0000
@@ -0,0 +1,42 @@
+########################################################################
+# Bug #711166: Partitioned tables are not correctly handled by the
+#              --databases and --tables-file options of innobackupex,
+#              and by the --tables option of xtrabackup.
+#              Testcase covers using --include option with InnoDB
+#              database and --stream mode
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server --innodb_file_per_table
+
+if ! ${MYSQL} ${MYSQL_ARGS} -e "SHOW VARIABLES LIKE '%partition%';" | grep YES ; then
+	echo "Requires partition support" > $SKIPPED_REASON
+	exit $SKIPPED_EXIT_CODE
+fi
+
+# Create MyISAM partitioned table
+ib_part_init $topdir MyISAM
+
+# Saving the checksum of original table
+checksum_a=`checksum_table test test`
+
+# Take a backup
+mkdir -p $topdir/backup
+innobackupex --stream=tar --include='test.test$' $topdir/backup > $topdir/backup/backup.tar
+$TAR ixvf $topdir/backup/backup.tar -C $topdir/backup 
+$TAR cvhf $topdir/backup/backup11.tar $mysql_datadir/test/*
+
+innobackupex --apply-log $topdir/backup
+
+vlog "Backup taken"
+
+stop_server
+
+# Restore partial backup
+ib_part_restore $topdir $mysql_datadir
+
+start_server
+
+ib_part_assert_checksum $checksum_a
 
=== added file 'test/t/ib_part_tf_innodb.sh'
--- test/t/ib_part_tf_innodb.sh	1970-01-01 00:00:00 +0000
+++ test/t/ib_part_tf_innodb.sh	2012-09-18 13:09:33 +0000
@@ -0,0 +1,49 @@
+########################################################################
+# Bug #711166: Partitioned tables are not correctly handled by the
+#              --databases and --tables-file options of innobackupex,
+#              and by the --tables option of xtrabackup.
+#              Testcase covers using --tables-file option with InnoDB
+#              database
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server --innodb_file_per_table
+
+if ! ${MYSQL} ${MYSQL_ARGS} -e "SHOW VARIABLES LIKE '%partition%';" | grep YES ; then
+	echo "Requires partition support" > $SKIPPED_REASON
+	exit $SKIPPED_EXIT_CODE
+fi
+
+# Create InnoDB partitioned table
+ib_part_init $topdir InnoDB
+
+# Saving the checksum of original table
+checksum_a=`checksum_table test test`
+
+# Take a backup
+# Only backup of test.test table will be taken
+cat >$topdir/tables <<EOF
+test.test
+EOF
+innobackupex --no-timestamp --tables-file=$topdir/tables $topdir/backup
+innobackupex --apply-log $topdir/backup
+vlog "Backup taken"
+
+COUNT=`xtrabackup --stats --tables-file=$topdir/tables --datadir=$topdir/backup 2>&1 \
+       | grep table: | awk '{print $2}' | sort -u | wc -l`
+
+if [ $COUNT != 5 ] ; then
+	vlog "xtrabackup --stats does not work"
+	exit -1
+fi
+
+stop_server
+
+# Restore partial backup
+ib_part_restore $topdir $mysql_datadir
+
+start_server
+
+ib_part_assert_checksum $checksum_a
 
=== added file 'test/t/ib_part_tf_innodb_stream.sh'
--- test/t/ib_part_tf_innodb_stream.sh	1970-01-01 00:00:00 +0000
+++ test/t/ib_part_tf_innodb_stream.sh	2012-09-18 13:09:33 +0000
@@ -0,0 +1,43 @@
+########################################################################
+# Bug #711166: Partitioned tables are not correctly handled by the
+#              --databases and --tables-file options of innobackupex,
+#              and by the --tables option of xtrabackup.
+#              Testcase covers using --tables-file option with InnoDB
+#              database and --stream mode
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server --innodb_file_per_table
+
+if ! ${MYSQL} ${MYSQL_ARGS} -e "SHOW VARIABLES LIKE '%partition%';" | grep YES ; then
+	echo "Requires partition support" > $SKIPPED_REASON
+	exit $SKIPPED_EXIT_CODE
+fi
+
+# Create InnoDB partitioned table
+ib_part_init $topdir InnoDB
+
+# Saving the checksum of original table
+checksum_a=`checksum_table test test`
+
+# Take a backup
+# Only backup of test.test table will be taken
+cat >$topdir/tables <<EOF
+test.test
+EOF
+mkdir -p $topdir/backup
+innobackupex --stream=tar --no-timestamp --tables-file=$topdir/tables $topdir/backup > $topdir/backup/backup.tar
+$TAR ixvf $topdir/backup/backup.tar -C $topdir/backup 
+innobackupex --apply-log $topdir/backup
+vlog "Backup taken"
+
+stop_server
+
+# Restore partial backup
+ib_part_restore $topdir $mysql_datadir
+
+start_server
+
+ib_part_assert_checksum $checksum_a
 
=== added file 'test/t/ib_part_tf_myisam.sh'
--- test/t/ib_part_tf_myisam.sh	1970-01-01 00:00:00 +0000
+++ test/t/ib_part_tf_myisam.sh	2012-09-18 13:09:33 +0000
@@ -0,0 +1,42 @@
+########################################################################
+# Bug #711166: Partitioned tables are not correctly handled by the
+#              --databases and --tables-file options of innobackupex,
+#              and by the --tables option of xtrabackup.
+#              Testcase covers using --tables-file option with MyISAM
+#              database
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server
+
+if ! ${MYSQL} ${MYSQL_ARGS} -e "SHOW VARIABLES LIKE '%partition%';" | grep YES ; then
+	echo "Requires partition support" > $SKIPPED_REASON
+	exit $SKIPPED_EXIT_CODE
+fi
+
+# Create MyISAM partitioned table with some partitions in
+# different location
+ib_part_init $topdir MyISAM
+
+# Saving the checksum of original table
+checksum_a=`checksum_table test test`
+
+# Take a backup
+# Only backup of test.test table will be taken
+cat >$topdir/tables <<EOF
+test.test
+EOF
+innobackupex --no-timestamp --tables-file=$topdir/tables $topdir/backup
+innobackupex --apply-log $topdir/backup
+vlog "Backup taken"
+
+stop_server
+
+# Restore partial backup
+ib_part_restore $topdir $mysql_datadir
+
+start_server
+
+ib_part_assert_checksum $checksum_a
 
=== modified file 'xtrabackup.c'
--- xtrabackup.c	2012-05-28 16:37:00 +0000
+++ xtrabackup.c	2012-09-18 13:09:33 +0000
@@ -576,6 +576,15 @@
 
 #endif /* INNODB_VERSION_SHORT */
 
+#ifdef INNODB_VERSION_SHORT
+#define XB_HASH_SEARCH(NAME, TABLE, FOLD, DATA, ASSERTION, TEST) \
+	HASH_SEARCH(NAME, TABLE, FOLD, xtrabackup_tables_t*, DATA, ASSERTION, \
+		    TEST)
+#else
+#define XB_HASH_SEARCH(NAME, TABLE, FOLD, DATA, ASSERTION, TEST) \
+	HASH_SEARCH(NAME, TABLE, FOLD, DATA, TEST)
+#endif
+
 typedef struct {
 	ulint	page_size;
 } xb_delta_info_t;
@@ -2518,6 +2527,73 @@
 	return(r);
 }
 
+static my_bool
+check_if_skip_table(const char *path, const char *suffix)
+{
+	char buf[FN_REFLEN];
+	const char *dbname, *tbname;
+	const char *ptr;
+	char *eptr;
+	int dbname_len;
+
+	if (xtrabackup_tables == NULL && xtrabackup_tables_file == NULL) {
+		return(FALSE);
+	}
+
+	dbname= NULL;
+	tbname= path;
+	while ((ptr= strstr(tbname, SRV_PATH_SEPARATOR_STR)) != NULL) {
+		dbname= tbname;
+		tbname= ptr + 1;
+	}
+
+	if (dbname == NULL) {
+		return(TRUE);
+	}
+
+	strncpy(buf, dbname, FN_REFLEN);
+	buf[FN_REFLEN - 1]= 0;
+	buf[tbname - 1 - dbname]= '.';
+
+	dbname_len= strlen(dbname) - strlen(suffix);
+	if (dbname_len < 1) {
+		return(TRUE);
+	}
+	buf[dbname_len - 1]= 0;
+
+	if ((eptr= strstr(buf, "#P")) != NULL) {
+		*eptr= 0;
+	}
+
+	if (xtrabackup_tables) {
+		int regres = REG_NOMATCH;
+		int i;
+		for (i = 0; i < tables_regex_num; i++) {
+			regres = regexec(&tables_regex[i], buf, 1,
+					    tables_regmatch, 0);
+			if (regres != REG_NOMATCH) {
+				break;
+			}
+		}
+		if (regres == REG_NOMATCH) {
+			return(TRUE);
+		}
+	}
+
+	if (xtrabackup_tables_file) {
+		xtrabackup_tables_t*	table;
+
+		XB_HASH_SEARCH(name_hash, tables_hash, ut_fold_string(buf),
+			       table, ut_ad(table->name),
+			       !strcmp(table->name, buf));
+		if (!table) {
+			return(TRUE);
+		}
+	}
+
+	return(FALSE);
+}
+
 /***********************************************************************
 Read meta info for an incremental delta.
 @return TRUE on success, FALSE on failure. */
@@ -2639,102 +2715,38 @@
 	info.page_size = 0;
 
 #ifdef XTRADB_BASED
-	if (xtrabackup_tables && (!trx_sys_sys_space(node->space->id)))
-#else
-	if (xtrabackup_tables && (node->space->id != 0))
-#endif
-	{ /* must backup id==0 */
-		char *p;
-		int p_len, regres= 0;
-		char *next, *prev;
-		char tmp;
-		int i;
-
-		p = node->name;
-		prev = NULL;
-		while ((next = strstr(p, SRV_PATH_SEPARATOR_STR)) != NULL)
-		{
-			prev = p;
-			p = next + 1;
-		}
-		p_len = strlen(p) - strlen(".ibd");
-
-		if (p_len < 1) {
-			/* unknown situation: skip filtering */
-			goto skip_filter;
-		}
-
-		/* TODO: Fix this lazy implementation... */
-		tmp = p[p_len];
-		p[p_len] = 0;
-		*(p - 1) = '.';
-
-		for (i = 0; i < tables_regex_num; i++) {
-			regres = regexec(&tables_regex[i], prev, 1, tables_regmatch, 0);
-			if (regres != REG_NOMATCH)
-				break;
-		}
-
-		p[p_len] = tmp;
-		*(p - 1) = SRV_PATH_SEPARATOR;
-
-		if ( regres == REG_NOMATCH ) {
-			printf("[%02u] Copying %s is skipped.\n",
-			       thread_n, node->name);
-			return(FALSE);
-		}
-	}
-
-#ifdef XTRADB_BASED
-	if (xtrabackup_tables_file && (!trx_sys_sys_space(node->space->id)))
-#else
-	if (xtrabackup_tables_file && (node->space->id != 0))
-#endif
-	{ /* must backup id==0 */
-		xtrabackup_tables_t* table;
-		char *p;
-		int p_len;
-		char *next, *prev;
-		char tmp;
-
-		p = node->name;
-		prev = NULL;
-		while ((next = strstr(p, SRV_PATH_SEPARATOR_STR)) != NULL)
-		{
-			prev = p;
-			p = next + 1;
-		}
-		p_len = strlen(p) - strlen(".ibd");
-
-		if (p_len < 1) {
-			/* unknown situation: skip filtering */
-			goto skip_filter;
-		}
-
-		/* TODO: Fix this lazy implementation... */
-		tmp = p[p_len];
-		p[p_len] = 0;
-
-		HASH_SEARCH(name_hash, tables_hash, ut_fold_string(prev),
-#ifdef INNODB_VERSION_SHORT
-			    xtrabackup_tables_t*,
-#endif
-			    table,
-#ifdef INNODB_VERSION_SHORT
-			    ut_ad(table->name),
-#endif
-	    		    !strcmp(table->name, prev));
-
-		p[p_len] = tmp;
-
-		if (!table) {
-			printf("[%02u] Copying %s is skipped.\n",
-			       thread_n, node->name);
-			return(FALSE);
-		}
-	}
-
-skip_filter:
+    if (xtrabackup_tables && (!trx_sys_sys_space(node->space->id))
+#else
+    if (xtrabackup_tables && (node->space->id != 0)
+#endif
+		&& check_if_skip_table(node->name, "ibd")) {
+		printf("[%02u] Copying %s is skipped.\n",
+		       thread_n, node->name);
+		return(FALSE);
+	}
+
+#ifndef INNODB_VERSION_SHORT
+	page_size = UNIV_PAGE_SIZE;
+	page_size_shift = UNIV_PAGE_SIZE_SHIFT;
+#else
+	zip_size = fil_space_get_zip_size(node->space->id);
+	if (zip_size == ULINT_UNDEFINED) {
+		goto skip;
+	} else if (zip_size) {
+		page_size = zip_size;
+		page_size_shift = get_bit_shift(page_size);
+		fprintf(stderr, "[%02u] %s is compressed with page size = "
+			"%lu bytes\n", thread_n, node->name, page_size);
+		if (page_size_shift < 10 || page_size_shift > 14) {
+			fprintf(stderr, "[%02u] xtrabackup: Error: Invalid "
+				"page size.\n", thread_n);
+			ut_error;
+		}
+	} else {
+		page_size = UNIV_PAGE_SIZE;
+		page_size_shift = UNIV_PAGE_SIZE_SHIFT;
+	}
+#endif
 
 #ifdef XTRADB_BASED
 	if (trx_sys_sys_space(node->space->id))
@@ -4610,47 +4622,8 @@
 		table = dict_table_get_low(table_name);
 		mem_free(table_name);
 
-
-		if (xtrabackup_tables) {
-			char *p;
-			int regres= 0;
-			int i;
-
-			p = strstr(table->name, SRV_PATH_SEPARATOR_STR);
-
-			if (p)
-				*p = '.';
-
-			for (i = 0; i < tables_regex_num; i++) {
-				regres = regexec(&tables_regex[i], table->name, 1, tables_regmatch, 0);
-				if (regres != REG_NOMATCH)
-					break;
-			}
-
-			if (p)
-				*p = SRV_PATH_SEPARATOR;
-
-			if ( regres == REG_NOMATCH )
-				goto skip;
-		}
-
-		if (xtrabackup_tables_file) {
-			xtrabackup_tables_t*	xtable;
-
-			HASH_SEARCH(name_hash, tables_hash, ut_fold_string(table->name),
-#ifdef INNODB_VERSION_SHORT
-				    xtrabackup_tables_t*,
-#endif
-				    xtable,
-#ifdef INNODB_VERSION_SHORT
-				    ut_ad(xtable->name),
-#endif
-				    !strcmp(xtable->name, table->name));
-
-			if (!xtable)
-				goto skip;
-		}
-
+		if (table && check_if_skip_table(table->name, ""))
+			goto skip;
 
 		if (table == NULL) {
 			fputs("InnoDB: Failed to load table ", stderr);
@@ -6174,12 +6147,6 @@
 				break;
 			}
 
-			while (*p != '\0') {
-				if (*p == '.') {
-					*p = '/';
-				}
-				p++;
-			}
 			p = strchr(name_buf, '\n');
 			if (p)
 			{