Skip to content

Commit

Permalink
dumpling: fix dumpling panic bug when --output-file-template is incor…
Browse files Browse the repository at this point in the history 
…rect (#41588)

close #42391
  • Loading branch information
@lichunzhu
lichunzhu committed Mar 21, 2023
1 parent 8ba2035 commit ca7e5f1
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 18 deletions.
9 changes: 2 additions & 7 deletions dumpling/export/ir_impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ func (td *tableData) Start(tctx *tcontext.Context, conn *sql.Conn) error {
if err = rows.Err(); err != nil {
return errors.Annotatef(err, "sql: %s", td.query)
}
td.SQLRowIter = nil
td.rows = rows
if td.needColTypes {
ns, err := rows.Columns()
Expand All @@ -227,14 +226,12 @@ func (td *tableData) Start(tctx *tcontext.Context, conn *sql.Conn) error {
td.colTypes = append(td.colTypes, c.DatabaseTypeName())
}
}
td.SQLRowIter = newRowIter(rows, td.colLen)

return nil
}

func (td *tableData) Rows() SQLRowIter {
if td.SQLRowIter == nil {
td.SQLRowIter = newRowIter(td.rows, td.colLen)
}
return td.SQLRowIter
}

Expand Down Expand Up @@ -354,13 +351,11 @@ func newMultiQueriesChunk(queries []string, colLength int) *multiQueriesChunk {
func (td *multiQueriesChunk) Start(tctx *tcontext.Context, conn *sql.Conn) error {
td.tctx = tctx
td.conn = conn
td.SQLRowIter = newMultiQueryChunkIter(td.tctx, td.conn, td.queries, td.colLen)
return nil
}

func (td *multiQueriesChunk) Rows() SQLRowIter {
if td.SQLRowIter == nil {
td.SQLRowIter = newMultiQueryChunkIter(td.tctx, td.conn, td.queries, td.colLen)
}
return td.SQLRowIter
}

Expand Down
37 changes: 26 additions & 11 deletions dumpling/tests/basic/run.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ DB_NAME="basic"
TABLE_NAME="t"
SEQUENCE_NAME="s"

# Test for simple case.
echo "Test for simple case."
run_sql "drop database if exists \`$DB_NAME\`;"
run_sql "create database \`$DB_NAME\` DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;"
run_sql "create table \`$DB_NAME\`.\`$TABLE_NAME\` (a int);"
Expand All @@ -26,7 +26,7 @@ cnt=$(grep -w "Welcome to dumpling.*Release Version.*Git Commit Hash.*Go Version
echo "version info count is ${cnt}"
[ "$cnt" = 1 ]

# Test for simple WHERE case.
echo "Test for simple WHERE case."
run_sql "drop database if exists \`$DB_NAME\`;"
run_sql "create database \`$DB_NAME\` DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;"
run_sql "create table \`$DB_NAME\`.\`$TABLE_NAME\` (a int);"
Expand All @@ -40,7 +40,7 @@ expected=$(seq 3 9)
echo "expected ${expected}, actual ${actual}"
[ "$actual" = "$expected" ]

# Test for OR WHERE case. Better dump MySQL here because Dumpling has some special handle for concurrently dump TiDB tables.
echo "Test for OR WHERE case." # Better dump MySQL here because Dumpling has some special handle for concurrently dump TiDB tables.
export DUMPLING_TEST_PORT=3306
run_sql "drop database if exists \`$DB_NAME\`;"
run_sql "create database \`$DB_NAME\` DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;"
Expand All @@ -61,7 +61,7 @@ echo "expected ${DUMPLING_OUTPUT_DIR}/${DB_NAME}.${TABLE_NAME}.000000009.sql ${e

seq 1 8 | xargs -I\? file_not_exist ${DUMPLING_OUTPUT_DIR}/${DB_NAME}.${TABLE_NAME}.00000000\?.sql

# Test for specifying --filetype sql with --sql, should report an error
echo "Test for specifying --filetype sql with --sql, should report an error."
set +e
run_dumpling --sql "select * from \`$DB_NAME\`.\`$TABLE_NAME\`" --filetype sql > ${DUMPLING_OUTPUT_DIR}/dumpling.log
set -e
Expand All @@ -70,8 +70,23 @@ actual=$(grep -w "unsupported config.FileType 'sql' when we specify --sql, pleas
echo "expected 1 return error when specifying --filetype sql and --sql, actual ${actual}"
[ "$actual" = 1 ]

echo "Test for incorrect --output-filename-template option causing panic issue."
set +e
run_dumpling --sql "select * from \`$DB_NAME\`.\`$TABLE_NAME\`" --filetype csv --output-filename-template "${TABLE_NAME}.${DB_NAME}.{{.index}}" > ${DUMPLING_OUTPUT_DIR}/dumpling.log
set -e

actual=$(grep -w "can't evaluate field index in type" ${DUMPLING_OUTPUT_DIR}/dumpling.log|wc -l)
echo "expected at least 1 return error when specifying --output-filename-template with {{.index}}, actual ${actual}"
[ "$actual" -ge 1 ]

echo "Test for --output-filename-template option."
run_dumpling --sql "select * from \`$DB_NAME\`.\`$TABLE_NAME\`" --filetype csv --output-filename-template "${TABLE_NAME}.${DB_NAME}.{{.Index}}"
cnt=$(cat ${DUMPLING_OUTPUT_DIR}/${TABLE_NAME}.${DB_NAME}.000000000.csv|wc -l)
echo "records count is ${cnt}"
[ "$cnt" = 101 ]

export DUMPLING_TEST_PORT=4000
# Test for --sql option.
echo "Test for --sql option."
run_sql "drop database if exists \`$DB_NAME\`;"
run_sql "create database \`$DB_NAME\` DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;"
run_sql "create sequence \`$DB_NAME\`.\`$SEQUENCE_NAME\` increment by 1;"
Expand All @@ -88,13 +103,13 @@ actual=$(sed -n '2p' ${DUMPLING_OUTPUT_DIR}/result.000000000.csv | sed "s/\r/r/
echo "expected 2r, actual ${actual}"
[ "$actual" = "2r" ]

# Test for dump with sequence
echo "Test for dump with sequence."
run_dumpling | tee ${DUMPLING_OUTPUT_DIR}/dumpling.log
actual=$(grep -w "dump failed" ${DUMPLING_OUTPUT_DIR}/dumpling.log|wc -l)
echo "expected 0, actual ${actual}"
[ "$actual" = 0 ]

# Test for tidb_mem_quota_query configuration
echo "Test for tidb_mem_quota_query configuration."
export GO_FAILPOINTS="github.com/pingcap/tidb/dumpling/export/PrintTiDBMemQuotaQuery=1*return"
run_dumpling | tee ${DUMPLING_OUTPUT_DIR}/dumpling.log
actual=$(grep -w "tidb_mem_quota_query == 1073741824" ${DUMPLING_OUTPUT_DIR}/dumpling.log|wc -l)
Expand All @@ -103,7 +118,7 @@ echo "expected 1, actual ${actual}"

export GO_FAILPOINTS=""

# Test for wrong sql causing panic problem: https://github.com/pingcap/dumpling/pull/234#issuecomment-759996695
echo "Test for wrong sql causing panic problem." # link issue: https://github.com/pingcap/dumpling/pull/234#issuecomment-759996695
set +e
run_dumpling --sql "test" > ${DUMPLING_OUTPUT_DIR}/dumpling.log 2> ${DUMPLING_OUTPUT_DIR}/dumpling.err
set -e
Expand All @@ -118,7 +133,7 @@ actual=$(grep -w "You have an error in your SQL syntax" ${DUMPLING_OUTPUT_DIR}/d
echo "expect contain error in SQL syntax, actual ${actual}"
[ "$actual" -ge 1 ]

# Test for consistency lock with empty database.
echo "Test for consistency lock with empty database."
export DUMPLING_TEST_PORT=3306
run_sql "drop database if exists \`$DB_NAME\`;"
run_sql "create database \`$DB_NAME\` DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;"
Expand All @@ -129,7 +144,7 @@ cnt=$(grep -w "$DB_NAME" ${DUMPLING_OUTPUT_DIR}/${DB_NAME}-schema-create.sql|wc
echo "records count is ${cnt}"
[ "$cnt" = 1 ]

# Test for recording network usage
echo "Test for recording network usage."
run_sql "drop database if exists test_db;"
run_sql "create database test_db;"
run_sql "create table test_db.test_table (a int primary key);"
Expand All @@ -138,4 +153,4 @@ run_sql "insert into test_db.test_table values (1),(2),(3),(4),(5),(6),(7),(8);"
export GO_FAILPOINTS="github.com/pingcap/tidb/dumpling/export/SetIOTotalBytes=return(1)"
run_dumpling -B "test_db" -L ${DUMPLING_OUTPUT_DIR}/dumpling.log
cnt=$(grep "IOTotalBytes=" ${DUMPLING_OUTPUT_DIR}/dumpling.log | grep -v "IOTotalBytes=0" | wc -l)
[ "$cnt" -ge 1 ]
[ "$cnt" -ge 1 ]

0 comments on commit ca7e5f1

Please sign in to comment.