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.

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

lightning: fix routes panic for csv data load #45405

Merged
merged 4 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions br/pkg/lightning/mydump/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,12 +517,30 @@
}
}
for _, info := range s.tableSchemas {
if _, ok := knownDBNames[info.TableName.Schema]; !ok {
knownDBNames[info.TableName.Schema] = &dbInfo{
fileMeta: info.FileMeta,
count: 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should use 1 or 0 here. Wrong value may cause problem in following logic. Can you add an integration test for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integration test added

}
}

Check warning on line 525 in br/pkg/lightning/mydump/loader.go

View check run for this annotation

Codecov / codecov/patch

br/pkg/lightning/mydump/loader.go#L521-L525

Added lines #L521 - L525 were not covered by tests
knownDBNames[info.TableName.Schema].count++
}
for _, info := range s.viewSchemas {
if _, ok := knownDBNames[info.TableName.Schema]; !ok {
knownDBNames[info.TableName.Schema] = &dbInfo{
fileMeta: info.FileMeta,
count: 1,
}
}

Check warning on line 534 in br/pkg/lightning/mydump/loader.go

View check run for this annotation

Codecov / codecov/patch

br/pkg/lightning/mydump/loader.go#L530-L534

Added lines #L530 - L534 were not covered by tests
knownDBNames[info.TableName.Schema].count++
}
for _, info := range s.tableDatas {
if _, ok := knownDBNames[info.TableName.Schema]; !ok {
knownDBNames[info.TableName.Schema] = &dbInfo{
fileMeta: info.FileMeta,
count: 1,
}
}
knownDBNames[info.TableName.Schema].count++
}

Expand Down
17 changes: 17 additions & 0 deletions br/pkg/lightning/mydump/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,23 @@ func TestRouter(t *testing.T) {
}
}

func TestRoutesPanic(t *testing.T) {
s := newTestMydumpLoaderSuite(t)
s.cfg.Routes = []*router.TableRule{
{
SchemaPattern: "test1",
TargetSchema: "test",
},
}

s.touch(t, "test1.dump_test.001.sql")
s.touch(t, "test1.dump_test.002.sql")
s.touch(t, "test1.dump_test.003.sql")

_, err := md.NewMyDumpLoader(context.Background(), s.cfg)
require.NoError(t, err)
}

func TestBadRouterRule(t *testing.T) {
s := newTestMydumpLoaderSuite(t)

Expand Down
9 changes: 9 additions & 0 deletions br/tests/lightning_routes_panic/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[tikv-importer]
backend = 'local'

# here we're verifying that routes does not panic for csv data load.
[[routes]]
schema-pattern = "test1"
table-pattern = "d*"
target-schema = "test"
target-table = "u"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
insert into dump_test values (1.0);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
insert into dump_test values (6.0);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
insert into dump_test values (36.0);
18 changes: 18 additions & 0 deletions br/tests/lightning_routes_panic/run.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/bin/sh

# Basic check for whether routing rules work

set -eux

run_sql 'DROP DATABASE IF EXISTS test1;'
run_sql 'DROP DATABASE IF EXISTS test;'

run_sql 'CREATE DATABASE test1;'
run_sql 'CREATE DATABASE test;'
run_sql 'CREATE TABLE test1.dump_test (x real primary key);'
run_sql 'CREATE TABLE test.u (x real primary key);'

run_lightning

run_sql 'SELECT sum(x) FROM test.u;'
check_contains 'sum(x): 43'
2 changes: 1 addition & 1 deletion br/tests/run_group.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ groups=(
["G12"]='lightning_drop_other_tables_halfway lightning_duplicate_detection lightning_duplicate_detection_new lightning_duplicate_resolution lightning_duplicate_resolution_incremental lightning_error_summary lightning_examples lightning_exotic_filenames lightning_extend_routes lightning_fail_fast'
["G13"]='lightning_fail_fast_on_nonretry_err lightning_file_routing lightning_foreign_key lightning_gcs lightning_generated_columns lightning_ignore_columns lightning_import_compress lightning_incremental lightning_issue_282'
["G14"]='lightning_issue_40657 lightning_issue_410 lightning_issue_519 lightning_local_backend lightning_max_incr lightning_max_random lightning_multi_valued_index lightning_new_collation lightning_no_schema'
["G15"]='lightning_parquet lightning_partition_incremental lightning_partitioned-table lightning_record_network lightning_reload_cert lightning_restore lightning_routes lightning_row-format-v2 lightning_s3'
["G15"]='lightning_parquet lightning_partition_incremental lightning_partitioned-table lightning_record_network lightning_reload_cert lightning_restore lightning_routes lightning_routes_panic lightning_row-format-v2 lightning_s3'
["G16"]='lightning_shard_rowid lightning_source_linkfile lightning_sqlmode lightning_tidb_duplicate_data lightning_tidb_rowid lightning_tiflash lightning_tikv_multi_rocksdb lightning_too_many_columns lightning_tool_135'
["G17"]='lightning_tool_1420 lightning_tool_1472 lightning_tool_241 lightning_ttl lightning_unused_config_keys lightning_various_types lightning_view lightning_write_batch lightning_write_limit'
)
Expand Down
Loading