From 906c188654941dc96b284aa1749cf2f58024173d Mon Sep 17 00:00:00 2001 From: dongmen <414110582@qq.com> Date: Tue, 7 Jan 2025 10:20:47 +0800 Subject: [PATCH 1/8] common: fix generate insert sql Signed-off-by: dongmen <414110582@qq.com> --- .github/workflows/integration_test_mysql.yaml | 2 +- pkg/common/table_info.go | 5 +++++ pkg/common/table_info_helper.go | 15 ++++++++++----- pkg/sink/mysql/mysql_writer.go | 1 + pkg/sink/mysql/sql_builder.go | 5 +++++ .../generate_column/data/virtual.sql | 7 ------- 6 files changed, 22 insertions(+), 13 deletions(-) diff --git a/.github/workflows/integration_test_mysql.yaml b/.github/workflows/integration_test_mysql.yaml index 7e29deb64e..467e26adce 100644 --- a/.github/workflows/integration_test_mysql.yaml +++ b/.github/workflows/integration_test_mysql.yaml @@ -30,7 +30,7 @@ jobs: # To boost the test speed, we split every 10 test cases into a group. e2e_test_group_1: runs-on: ubuntu-latest - name: E2E Test + name: E2E Test Group 1 steps: - name: Check out code uses: actions/checkout@v2 diff --git a/pkg/common/table_info.go b/pkg/common/table_info.go index 43bef1afeb..1999395c1b 100644 --- a/pkg/common/table_info.go +++ b/pkg/common/table_info.go @@ -74,6 +74,11 @@ const ( UnsignedFlag ) +func NewColumnFlagType(flag ColumnFlagType) *ColumnFlagType { + f := ColumnFlagType(flag) + return &f +} + // SetIsBinary sets BinaryFlag func (b *ColumnFlagType) SetIsBinary() { (*Flag)(b).Add(Flag(BinaryFlag)) diff --git a/pkg/common/table_info_helper.go b/pkg/common/table_info_helper.go index 3d39b77023..f8ad199d4e 100644 --- a/pkg/common/table_info_helper.go +++ b/pkg/common/table_info_helper.go @@ -751,12 +751,13 @@ func (s *columnSchema) genPreSQLInsert(isReplace bool, needPlaceHolder bool) str builder.WriteString("INSERT INTO %s") } builder.WriteString(" (") - builder.WriteString(s.getColumnList(false)) + nonGeneratedColumnCount, columnList := s.getColumnList(false) + builder.WriteString(columnList) builder.WriteString(") VALUES ") if needPlaceHolder { builder.WriteString("(") - builder.WriteString(placeHolder(len(s.Columns) - s.VirtualColumnCount)) + builder.WriteString(placeHolder(nonGeneratedColumnCount)) builder.WriteString(")") } return builder.String() @@ -766,7 +767,8 @@ func (s *columnSchema) genPreSQLUpdate() string { var builder strings.Builder builder.WriteString("UPDATE %s") builder.WriteString(" SET ") - builder.WriteString(s.getColumnList(true)) + _, columnList := s.getColumnList(true) + builder.WriteString(columnList) return builder.String() } @@ -784,12 +786,15 @@ func placeHolder(n int) string { return builder.String() } -func (s *columnSchema) getColumnList(isUpdate bool) string { +// getColumnList returns non-generated columns number and column names +func (s *columnSchema) getColumnList(isUpdate bool) (int, string) { var b strings.Builder + nonGeneratedColumnCount := 0 for i, col := range s.Columns { if col == nil || s.ColumnsFlag[col.ID].IsGeneratedColumn() { continue } + nonGeneratedColumnCount++ if i > 0 { b.WriteString(",") } @@ -798,5 +803,5 @@ func (s *columnSchema) getColumnList(isUpdate bool) string { b.WriteString(" = ?") } } - return b.String() + return nonGeneratedColumnCount, b.String() } diff --git a/pkg/sink/mysql/mysql_writer.go b/pkg/sink/mysql/mysql_writer.go index 83046e2cf1..e967ec54d6 100644 --- a/pkg/sink/mysql/mysql_writer.go +++ b/pkg/sink/mysql/mysql_writer.go @@ -753,6 +753,7 @@ func (w *MysqlWriter) prepareDMLs(events []*commonEvent.DMLEvent) (*preparedDMLs func (w *MysqlWriter) Flush(events []*commonEvent.DMLEvent) error { dmls, err := w.prepareDMLs(events) + log.Info("fizz prepareDMLs", zap.Any("dmls", dmls.String()), zap.Any("events", events)) if err != nil { return errors.Trace(err) } diff --git a/pkg/sink/mysql/sql_builder.go b/pkg/sink/mysql/sql_builder.go index 091830e325..e86e2a841e 100644 --- a/pkg/sink/mysql/sql_builder.go +++ b/pkg/sink/mysql/sql_builder.go @@ -14,6 +14,7 @@ package mysql import ( + "fmt" "strings" "sync" @@ -33,6 +34,10 @@ type preparedDMLs struct { startTs []uint64 } +func (d *preparedDMLs) String() string { + return fmt.Sprintf("sqls: %v, values: %v, rowCount: %d, approximateSize: %d, startTs: %v", d.sqls, d.values, d.rowCount, d.approximateSize, d.startTs) +} + var dmlsPool = sync.Pool{ New: func() interface{} { return &preparedDMLs{ diff --git a/tests/integration_tests/generate_column/data/virtual.sql b/tests/integration_tests/generate_column/data/virtual.sql index 789bc4d744..b3dea88123 100644 --- a/tests/integration_tests/generate_column/data/virtual.sql +++ b/tests/integration_tests/generate_column/data/virtual.sql @@ -2,13 +2,6 @@ drop database if exists `generate_column`; create database `generate_column`; use `generate_column`; -create table t (a int, b int as (a + 1) stored primary key); -insert into t(a) values (1),(2), (3),(4),(5),(6),(7); -update t set a = 10 where a = 1; -update t set a = 11 where b = 3; -delete from t where b=4; -delete from t where a=4; - create table t1 (a int, b int as (a + 1) virtual not null, c int not null, unique index idx1(b), unique index idx2(c)); insert into t1 (a, c) values (1, 2),(2, 3), (3, 4),(4, 5),(5, 6),(6, 7),(7, 8); update t1 set a = 10 where a = 1; From 06193100742dcaa4e1aa485bd7b074c4be0ed636 Mon Sep 17 00:00:00 2001 From: dongmen <414110582@qq.com> Date: Tue, 7 Jan 2025 10:21:01 +0800 Subject: [PATCH 2/8] common: add unit test Signed-off-by: dongmen <414110582@qq.com> --- pkg/common/table_info_helper_test.go | 96 ++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 pkg/common/table_info_helper_test.go diff --git a/pkg/common/table_info_helper_test.go b/pkg/common/table_info_helper_test.go new file mode 100644 index 0000000000..6d6e8ee230 --- /dev/null +++ b/pkg/common/table_info_helper_test.go @@ -0,0 +1,96 @@ +package common + +import ( + "testing" + + "github.com/pingcap/tidb/pkg/meta/model" + pmodel "github.com/pingcap/tidb/pkg/parser/model" + "github.com/stretchr/testify/require" +) + +func TestColumnSchema_GetColumnList(t *testing.T) { + tests := []struct { + name string + columns []*model.ColumnInfo + columnsFlag map[int64]*ColumnFlagType + isUpdate bool + wantCount int + wantColumnList string + }{ + { + name: "normal columns without update", + columns: []*model.ColumnInfo{ + {Name: pmodel.CIStr{O: "id", L: "id"}, ID: 1}, + {Name: pmodel.CIStr{O: "name", L: "name"}, ID: 2}, + {Name: pmodel.CIStr{O: "age", L: "age"}, ID: 3}, + }, + columnsFlag: map[int64]*ColumnFlagType{ + 1: NewColumnFlagType(PrimaryKeyFlag), + 2: NewColumnFlagType(UniqueKeyFlag), + 3: NewColumnFlagType(NullableFlag), + }, + isUpdate: false, + wantCount: 3, + wantColumnList: "`id`,`name`,`age`", + }, + { + name: "normal columns with update", + columns: []*model.ColumnInfo{ + {Name: pmodel.CIStr{O: "id", L: "id"}, ID: 1}, + {Name: pmodel.CIStr{O: "name", L: "name"}, ID: 2}, + {Name: pmodel.CIStr{O: "age", L: "age"}, ID: 3}, + }, + columnsFlag: map[int64]*ColumnFlagType{ + 1: NewColumnFlagType(PrimaryKeyFlag), + 2: NewColumnFlagType(UniqueKeyFlag), + 3: NewColumnFlagType(NullableFlag), + }, + isUpdate: true, + wantCount: 3, + wantColumnList: "`id` = ?,`name` = ?,`age` = ?", + }, + { + name: "with generated columns", + columns: []*model.ColumnInfo{ + {Name: pmodel.CIStr{O: "id", L: "id"}, ID: 1}, + {Name: pmodel.CIStr{O: "name", L: "name"}, ID: 2}, + {Name: pmodel.CIStr{O: "full_name", L: "full_name"}, ID: 3}, // generated column + }, + columnsFlag: map[int64]*ColumnFlagType{ + 1: NewColumnFlagType(PrimaryKeyFlag), + 2: NewColumnFlagType(UniqueKeyFlag), + 3: NewColumnFlagType(GeneratedColumnFlag), + }, + isUpdate: false, + wantCount: 2, + wantColumnList: "`id`,`name`", + }, + { + name: "with nil column", + columns: []*model.ColumnInfo{ + {Name: pmodel.CIStr{O: "id", L: "id"}, ID: 1}, + nil, + {Name: pmodel.CIStr{O: "age", L: "age"}, ID: 3}, + }, + columnsFlag: map[int64]*ColumnFlagType{ + 1: NewColumnFlagType(PrimaryKeyFlag), + 3: NewColumnFlagType(NullableFlag), + }, + isUpdate: false, + wantCount: 2, + wantColumnList: "`id`,`age`", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &columnSchema{ + Columns: tt.columns, + ColumnsFlag: tt.columnsFlag, + } + gotCount, gotColumnList := s.getColumnList(tt.isUpdate) + require.Equal(t, tt.wantCount, gotCount) + require.Equal(t, tt.wantColumnList, gotColumnList) + }) + } +} From 07d3d515ee2fedd6fc99c5b391408560c950b13e Mon Sep 17 00:00:00 2001 From: dongmen <414110582@qq.com> Date: Tue, 7 Jan 2025 10:30:46 +0800 Subject: [PATCH 3/8] mysql: add debug log to help testing Signed-off-by: dongmen <414110582@qq.com> --- pkg/sink/mysql/mysql_writer.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/sink/mysql/mysql_writer.go b/pkg/sink/mysql/mysql_writer.go index e967ec54d6..0b3e0d4b67 100644 --- a/pkg/sink/mysql/mysql_writer.go +++ b/pkg/sink/mysql/mysql_writer.go @@ -39,6 +39,7 @@ import ( "github.com/pingcap/tiflow/pkg/retry" pmysql "github.com/pingcap/tiflow/pkg/sink/mysql" "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) const ( @@ -748,12 +749,17 @@ func (w *MysqlWriter) prepareDMLs(events []*commonEvent.DMLEvent) (*preparedDMLs } } + // Pre-check log level to avoid dmls.String() being called unnecessarily + // This method is expensive, so we only log it when the log level is debug. + if log.GetLevel() == zapcore.DebugLevel { + log.Debug("prepareDMLs", zap.Any("dmls", dmls.String()), zap.Any("events", events)) + } + return dmls, nil } func (w *MysqlWriter) Flush(events []*commonEvent.DMLEvent) error { dmls, err := w.prepareDMLs(events) - log.Info("fizz prepareDMLs", zap.Any("dmls", dmls.String()), zap.Any("events", events)) if err != nil { return errors.Trace(err) } From 7908b5a636d53911568bbce1eddd1048d3f09db4 Mon Sep 17 00:00:00 2001 From: dongmen <414110582@qq.com> Date: Tue, 7 Jan 2025 10:36:08 +0800 Subject: [PATCH 4/8] ci: only triger ci on non-draft PR Signed-off-by: dongmen <414110582@qq.com> --- .github/workflows/check_and_build.yaml | 1 + .github/workflows/integration_test_mysql.yaml | 1 + .github/workflows/uint_test.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/.github/workflows/check_and_build.yaml b/.github/workflows/check_and_build.yaml index a4c4879bb4..7011755f10 100644 --- a/.github/workflows/check_and_build.yaml +++ b/.github/workflows/check_and_build.yaml @@ -12,6 +12,7 @@ on: - 'OWNERS_ALIASES' pull_request: + types: [opened, synchronize, reopened, ready_for_review] branches: - master - "release-[0-9].[0-9]*" diff --git a/.github/workflows/integration_test_mysql.yaml b/.github/workflows/integration_test_mysql.yaml index 467e26adce..2127b15ff7 100644 --- a/.github/workflows/integration_test_mysql.yaml +++ b/.github/workflows/integration_test_mysql.yaml @@ -12,6 +12,7 @@ on: - 'OWNERS_ALIASES' pull_request: + types: [opened, synchronize, reopened, ready_for_review] branches: - master - "release-[0-9].[0-9]*" diff --git a/.github/workflows/uint_test.yaml b/.github/workflows/uint_test.yaml index d4ee537cd4..0173ff6fa7 100644 --- a/.github/workflows/uint_test.yaml +++ b/.github/workflows/uint_test.yaml @@ -12,6 +12,7 @@ on: - 'OWNERS_ALIASES' pull_request: + types: [opened, synchronize, reopened, ready_for_review] branches: - master - "release-[0-9].[0-9]*" From 30237d3c41a811f234f4e746c43e264cf1e52c4c Mon Sep 17 00:00:00 2001 From: dongmen <414110582@qq.com> Date: Tue, 7 Jan 2025 10:50:28 +0800 Subject: [PATCH 5/8] github: add PR template Signed-off-by: dongmen <414110582@qq.com> --- .github/pull_request_template.md | 41 ++++++++++++++++ CONTRIBUTING.md | 81 ++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 .github/pull_request_template.md create mode 100644 CONTRIBUTING.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000000..898f564e3c --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,41 @@ + + +### What problem does this PR solve? + + +Issue Number: close #xxx + +### What is changed and how it works? + +### Check List + +#### Tests + +- Unit test +- Integration test +- Manual test (add detailed scripts or steps below) +- No code + +#### Questions + +##### Will it cause performance regression or break compatibility? + +##### Do you need to update user documentation, design documentation or monitoring documentation? + +### Release note + +```release-note +Please refer to [Release Notes Language Style Guide](https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/release-notes-style-guide.html) to write a quality release note. + +If you don't think this PR needs a release note then fill it with `None`. +``` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..788219a61a --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,81 @@ +# How to contribute + +This document outlines some of the conventions on development workflow, commit +message formatting, contact points and other resources to make it easier to get +your contribution accepted. + +## Get started + +- Fork the repository on GitHub. +- Read the README.md for build instructions. +- Play with the project, submit bugs, submit patches! + +## Build TiCDC + +Developing TiCDC requires: + +* [Go 1.23+](https://go.dev/doc/code) +* An internet connection to download the dependencies + +Simply run `make cdc` to build the program. + +```sh +make cdc +``` + +### Run tests + +TODO + +### Update dependencies + +TiCDC uses [Go Modules](https://github.com/golang/go/wiki/Modules) to manage dependencies. To add or update a dependency: use the `go mod edit` command to change the dependency. + +## Contribution flow + +This is a rough outline of what a contributor's workflow looks like: + +- Create a topic branch from where you want to base your work. This is usually `master`. +- Make commits of logical units and add test case if the change fixes a bug or adds new functionality. +- Run tests and make sure all the tests are passed. +- Make sure your commit messages are in the proper format (see below). +- Push your changes to a topic branch in your fork of the repository. +- Submit a pull request. +- Your PR must receive LGTMs from two maintainers. + +Thanks for your contributions! + +### Code style + +The coding style suggested by the Golang community is used in TiCDC. See the [style doc](https://github.com/golang/go/wiki/CodeReviewComments) for details. + +Please follow this style to make TiCDC easy to review, maintain and develop. + +### Commit message format + +We follow a rough convention for commit messages that is designed to answer two +questions: what changed and why. The subject line should feature the what and +the body of the commit should describe the why. + +```shell +maintainer: add comment for variable declaration + +Improve documentation. +``` + +The format can be described more formally as follows: + +```shell +: + + + +