diff --git a/.CHANGELOG.md b/.CHANGELOG.md index 365e9b5..999c1a8 100644 --- a/.CHANGELOG.md +++ b/.CHANGELOG.md @@ -40,6 +40,7 @@ - [feat(merger): 定义中立的特征表达数据、定义工厂方法根据特征数据来获取具体的merger](https://github.com/ecodeclub/eorm/pull/222) - [refactor(merger): 重构AVG函数实现,重构所有rows.Rows实现的ConlumnType方法并添加测试](https://github.com/ecodeclub/eorm/pull/223) - [feat(merger): 新增Distinct Merger](https://github.com/ecodeclub/eorm/pull/224) +- [refactor(merger): 去掉无用代码及过期注释,整理代码](https://github.com/ecodeclub/eorm/pull/225) ## v0.0.1: - [Init Project](https://github.com/ecodeclub/eorm/pull/1) - [Selector Definition](https://github.com/ecodeclub/eorm/pull/2) diff --git a/internal/merger/factory/factory.go b/internal/merger/factory/factory.go index 111894b..22ab6a1 100644 --- a/internal/merger/factory/factory.go +++ b/internal/merger/factory/factory.go @@ -34,12 +34,12 @@ import ( ) var ( - ErrInvalidColumnInfo = errors.New("factory: ColumnInfo非法") - ErrEmptyColumnList = errors.New("factory: 列列表为空") - ErrColumnNotFoundInSelectList = errors.New("factory: Select列表中未找到列") - ErrInvalidLimit = errors.New("factory: Limit小于1") - ErrInvalidOffset = errors.New("factory: Offset不等于0") - ErrInvalidFeatures = errors.New("factory: Features非法") + ErrInvalidColumnInfo = errors.New("merger: ColumnInfo非法") + ErrEmptyColumnList = errors.New("merger: 列列表为空") + ErrColumnNotFoundInSelectList = errors.New("merger: Select列表中未找到列") + ErrInvalidLimit = errors.New("merger: Limit小于1") + ErrInvalidOffset = errors.New("merger: Offset不等于0") + ErrInvalidFeatures = errors.New("merger: Features非法") ) type ( @@ -62,31 +62,19 @@ type ( ) func (q QuerySpec) Validate() error { - - if err := q.validateFeatures(); err != nil { - return err - } - - if err := q.validateSelect(); err != nil { - return err - } - - if err := q.validateGroupBy(); err != nil { - return err - } - - if err := q.validateDistinct(); err != nil { - return err - } - - if err := q.validateOrderBy(); err != nil { - return err - } - - if err := q.validateLimit(); err != nil { - return err + validateFuncs := []func() error{ + q.validateFeatures, + q.validateSelect, + q.validateGroupBy, + q.validateDistinct, + q.validateOrderBy, + q.validateLimit, + } + for _, f := range validateFuncs { + if err := f(); err != nil { + return err + } } - return nil } @@ -199,6 +187,51 @@ func (q QuerySpec) validateLimit() error { return nil } +// New 根据原SQL查询特征、目标SQL查询特征创建、组合merger的工厂方法 +func New(origin, target QuerySpec) (merger.Merger, error) { + for _, spec := range []QuerySpec{origin, target} { + if err := spec.Validate(); err != nil { + return nil, err + } + } + var mp = map[query.Feature]newMergerFunc{ + query.AggregateFunc: newAggregateMerger, + query.GroupBy: newGroupByMergerWithoutHaving, + query.Distinct: newDistinctMerger, + query.OrderBy: newOrderByMerger, + } + var mergers []merger.Merger + for _, feature := range target.Features { + switch feature { + case query.AggregateFunc, query.GroupBy, query.Distinct, query.OrderBy: + m, err := mp[feature](origin, target) + if err != nil { + return nil, err + } + mergers = append(mergers, m) + case query.Limit: + var prev merger.Merger + if len(mergers) == 0 { + prev = batchmerger.NewMerger() + } else { + prev = mergers[len(mergers)-1] + mergers = mergers[:len(mergers)-1] + } + m, err := pagedmerger.NewMerger(prev, target.Offset, target.Limit) + if err != nil { + return nil, err + } + mergers = append(mergers, m) + default: + return nil, fmt.Errorf("%w: feature: %d", ErrInvalidFeatures, feature) + } + } + if len(mergers) == 0 { + mergers = append(mergers, batchmerger.NewMerger()) + } + return &pipeline{mergers: mergers}, nil +} + func newAggregateMerger(origin, target QuerySpec) (merger.Merger, error) { aggregators := getAggregators(origin, target) // TODO: 当aggs为空时, 报不相关的错 merger: scan之前需要调用Next @@ -262,55 +295,11 @@ func newOrderByMerger(origin, target QuerySpec) (merger.Merger, error) { return sortmerger.NewMerger(isPreScanAll, columns...) } -func New(origin, target QuerySpec) (merger.Merger, error) { - for _, spec := range []QuerySpec{origin, target} { - if err := spec.Validate(); err != nil { - return nil, err - } - } - var mp = map[query.Feature]newMergerFunc{ - query.AggregateFunc: newAggregateMerger, - query.GroupBy: newGroupByMergerWithoutHaving, - query.Distinct: newDistinctMerger, - query.OrderBy: newOrderByMerger, - } - var mergers []merger.Merger - for _, feature := range target.Features { - switch feature { - case query.AggregateFunc, query.GroupBy, query.Distinct, query.OrderBy: - m, err := mp[feature](origin, target) - if err != nil { - return nil, err - } - mergers = append(mergers, m) - case query.Limit: - var prev merger.Merger - if len(mergers) == 0 { - prev = batchmerger.NewMerger() - } else { - prev = mergers[len(mergers)-1] - mergers = mergers[:len(mergers)-1] - } - m, err := pagedmerger.NewMerger(prev, target.Offset, target.Limit) - if err != nil { - return nil, err - } - mergers = append(mergers, m) - default: - return nil, fmt.Errorf("%w: feature: %d", ErrInvalidFeatures, feature) - } - } - if len(mergers) == 0 { - mergers = append(mergers, batchmerger.NewMerger()) - } - return &MergerPipeline{mergers: mergers}, nil -} - -type MergerPipeline struct { +type pipeline struct { mergers []merger.Merger } -func (m *MergerPipeline) Merge(ctx context.Context, results []rows.Rows) (rows.Rows, error) { +func (m *pipeline) Merge(ctx context.Context, results []rows.Rows) (rows.Rows, error) { r, err := m.mergers[0].Merge(ctx, results) if err != nil { return nil, err @@ -327,7 +316,7 @@ func (m *MergerPipeline) Merge(ctx context.Context, results []rows.Rows) (rows.R return r, nil } -// NewBatchMerger 仅供sharding_select通过测试使用,后续重构并删掉该方法并只保留上方New方法 +// NewBatchMerger 仅供sharding_select.go使用,后续重构后需要删掉该方法并只保留上方New方法 func NewBatchMerger() (merger.Merger, error) { return batchmerger.NewMerger(), nil } diff --git a/internal/merger/factory/factory_test.go b/internal/merger/factory/factory_test.go index d8d00a5..90830d3 100644 --- a/internal/merger/factory/factory_test.go +++ b/internal/merger/factory/factory_test.go @@ -21,360 +21,12 @@ import ( "github.com/DATA-DOG/go-sqlmock" "github.com/ecodeclub/eorm/internal/merger" - "github.com/ecodeclub/eorm/internal/merger/internal/aggregatemerger" - "github.com/ecodeclub/eorm/internal/merger/internal/batchmerger" - "github.com/ecodeclub/eorm/internal/merger/internal/groupbymerger" - "github.com/ecodeclub/eorm/internal/merger/internal/pagedmerger" - "github.com/ecodeclub/eorm/internal/merger/internal/sortmerger" "github.com/ecodeclub/eorm/internal/query" "github.com/ecodeclub/eorm/internal/rows" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) -func TestNew(t *testing.T) { - t.Skip() - // TODO: 本测试为列探索测试用例,以后会删掉 - tests := []struct { - name string - sql string - spec QuerySpec - - wantMergers []merger.Merger - requireErrFunc require.ErrorAssertionFunc - }{ - // 单一特征的测试用例 - { - name: "无特征_使用批量合并", - sql: "SELECT `id`,`status` FROM `orders`", - spec: QuerySpec{ - Features: nil, - Select: []merger.ColumnInfo{ - { - Index: 0, - Name: "id", - }, - { - Index: 1, - Name: "status", - }, - }, - }, - wantMergers: []merger.Merger{ - &batchmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "SELECT中有别名_使用批量合并", - sql: "SELECT `id` AS `order_id`, `user_id` AS `uid`, `order_sn` AS `sn`, `amount`, `status`, COUNT(*) AS `total_orders`, SUM(`amount`) AS `total_amount`, AVG(`amount`) AS `avg_amount` FROM `orders` WHERE (`status` = 1 AND `amount` > 100) OR `amount` > 1000;", - spec: QuerySpec{ - Features: nil, - Select: []merger.ColumnInfo{ - { - Index: 0, - Name: "id", - }, - { - Index: 1, - Name: "status", - }, - }, - }, - wantMergers: []merger.Merger{ - &batchmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - // SELECT中有聚合函数_使用 - { - name: "有聚合函数_使用聚合合并", - sql: "SELECT COUNT(`id`) FROM `orders`", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc}, - // TODO: 初始化aggregatemerger时,要从select中读取参数 - Select: []merger.ColumnInfo{ - { - Index: 0, - Name: "COUNT(id)", - }, - }, - }, - wantMergers: []merger.Merger{ - &aggregatemerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "有GroupBy_无Having_GroupBy无分片键_使用分组聚合合并", - sql: "SELECT `amount` FROM `orders` GROUP BY `amount`", - spec: QuerySpec{ - Features: []query.Feature{query.GroupBy}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - }, - wantMergers: []merger.Merger{ - &groupbymerger.AggregatorMerger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "有GroupBy_无Having_GroupBy中有分片键_使用分组聚合合并", - sql: "SELECT AVG(`amount`) FROM `orders` GROUP BY `buyer_id`", - spec: QuerySpec{ - Features: []query.Feature{query.GroupBy}, - Select: []merger.ColumnInfo{ - { - Index: 0, - Name: "AVG(`amount`)", - AggregateFunc: "AVG", // isAggregateFunc ? - }, - }, - // TOTO: GroupBy - GroupBy: []merger.ColumnInfo{{Name: "buyer_id"}}, - }, - wantMergers: []merger.Merger{ - &groupbymerger.AggregatorMerger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "OrderBy", - sql: "SELECT `sn` FROM `orders` ORDER BY `amount`", - spec: QuerySpec{ - Features: []query.Feature{query.OrderBy}, - Select: []merger.ColumnInfo{ - { - Index: 0, - Name: "sn", - }, - }, - OrderBy: []merger.ColumnInfo{ - { - Index: 0, // 索引排序? amount没有出现在SELECT子句,出现在orderBy子句中 - Name: "amount", - Order: merger.OrderASC, - }, - }, - }, - wantMergers: []merger.Merger{ - &sortmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "Limit", - sql: "SELECT `name` FROM `orders` LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.Limit}, - // Select: []merger.ColumnInfo{{Index: 0, Name: "name"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - // 组合特征的测试用例 - { - name: "AggregateFunc_GroupBy", - sql: "SELECT `amount`, COUNT(*) FROM `orders` GROUP BY `amount`", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc, query.GroupBy}, - Select: []merger.ColumnInfo{{Name: "amount"}, {Name: "COUNT(*)"}}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - }, - wantMergers: []merger.Merger{ - &aggregatemerger.Merger{}, - &groupbymerger.AggregatorMerger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "AggregateFunc_OrderBy", - sql: "SELECT COUNT(*) FROM `orders` ORDER BY `amount`", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc, query.OrderBy}, - Select: []merger.ColumnInfo{{Name: "COUNT(*)"}}, - OrderBy: []merger.ColumnInfo{{Name: "amount"}}, - }, - wantMergers: []merger.Merger{ - &aggregatemerger.Merger{}, - &sortmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "AggregateFunc_Limit", - sql: "SELECT COUNT(*) FROM `orders` LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc, query.Limit}, - Select: []merger.ColumnInfo{{Name: "COUNT(*)"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - // &aggregatemerger.Merger{}, - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "GroupBy_OrderBy", - sql: "SELECT `amount` FROM `orders` GROUP BY `amount` ORDER BY `amount`", - spec: QuerySpec{ - Features: []query.Feature{query.GroupBy, query.OrderBy}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - OrderBy: []merger.ColumnInfo{{Name: "amount"}}, - }, - wantMergers: []merger.Merger{ - &groupbymerger.AggregatorMerger{}, - &sortmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "GroupBy_Limit", - sql: "SELECT `amount` FROM `orders` GROUP BY `amount` LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.GroupBy, query.Limit}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - // &groupbymerger.AggregatorMerger{}, - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "OrderBy_Limit", - sql: "SELECT `name` FROM `orders` ORDER BY `amount` LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.OrderBy, query.Limit}, - OrderBy: []merger.ColumnInfo{{Name: "amount"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - // &sortmerger.Merger{}, - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "AggregateFunc_GroupBy_OrderBy", - sql: "SELECT `amount`, COUNT(*) FROM `orders` GROUP BY `amount` ORDER BY COUNT(*)", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc, query.GroupBy, query.OrderBy}, - Select: []merger.ColumnInfo{{Name: "amount"}, {Name: "COUNT(*)"}}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - OrderBy: []merger.ColumnInfo{{Name: "COUNT(*)"}}, - }, - wantMergers: []merger.Merger{ - &aggregatemerger.Merger{}, - &groupbymerger.AggregatorMerger{}, - &sortmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "AggregateFunc_GroupBy_Limit", - sql: "SELECT `amount`, COUNT(*) FROM `orders` GROUP BY `amount` LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc, query.GroupBy, query.Limit}, - Select: []merger.ColumnInfo{{Name: "amount"}, {Name: "COUNT(*)"}}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - &aggregatemerger.Merger{}, - // &groupbymerger.AggregatorMerger{}, - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "AggregateFunc_OrderBy_Limit", - sql: "SELECT COUNT(*) FROM `orders` ORDER BY `amount` LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc, query.OrderBy, query.Limit}, - Select: []merger.ColumnInfo{{Name: "COUNT(*)"}}, - OrderBy: []merger.ColumnInfo{{Name: "amount"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - &aggregatemerger.Merger{}, - // &sortmerger.Merger{}, - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "GroupBy_OrderBy_Limit", - sql: "SELECT `amount` FROM `orders` GROUP BY `amount` ORDER BY `amount` LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.GroupBy, query.OrderBy, query.Limit}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - OrderBy: []merger.ColumnInfo{{Name: "amount"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - &groupbymerger.AggregatorMerger{}, - // &sortmerger.Merger{}, - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - { - name: "AggregateFunc_GroupBy_OrderBy_Limit", - sql: "SELECT `amount`, COUNT(*) FROM `orders` GROUP BY `amount` ORDER BY COUNT(*) LIMIT 10", - spec: QuerySpec{ - Features: []query.Feature{query.AggregateFunc, query.GroupBy, query.OrderBy, query.Limit}, - Select: []merger.ColumnInfo{{Name: "amount"}, {Name: "COUNT(*)"}}, - GroupBy: []merger.ColumnInfo{{Name: "amount"}}, - OrderBy: []merger.ColumnInfo{{Name: "COUNT(*)"}}, - Limit: 10, - }, - wantMergers: []merger.Merger{ - &aggregatemerger.Merger{}, - &groupbymerger.AggregatorMerger{}, - // &sortmerger.Merger{}, - &pagedmerger.Merger{}, - }, - requireErrFunc: require.NoError, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - - m, err := New(tt.spec, tt.spec) - tt.requireErrFunc(t, err) - - mp, ok := m.(*MergerPipeline) - require.True(t, ok) - - // Ensure the number of mergers match - assert.Equal(t, len(tt.wantMergers), len(mp.mergers)) - - // Ensure each merger matches the expected order and type - for i, expectedMerger := range tt.wantMergers { - switch expectedMerger.(type) { - case *batchmerger.Merger: - assert.IsType(t, &batchmerger.Merger{}, mp.mergers[i]) - case *aggregatemerger.Merger: - assert.IsType(t, &aggregatemerger.Merger{}, mp.mergers[i]) - case *groupbymerger.AggregatorMerger: - assert.IsType(t, &groupbymerger.AggregatorMerger{}, mp.mergers[i]) - case *sortmerger.Merger: - assert.IsType(t, &sortmerger.Merger{}, mp.mergers[i]) - case *pagedmerger.Merger: - assert.IsType(t, &pagedmerger.Merger{}, mp.mergers[i]) - } - } - }) - } -} - func TestFactory(t *testing.T) { suite.Run(t, &factoryTestSuite{}) } @@ -407,7 +59,7 @@ func (s *factoryTestSuite) TearDownTest() { s.NoError(s.mock03.ExpectationsWereMet()) } -func (s *factoryTestSuite) TestSELECT() { +func (s *factoryTestSuite) TestNewAndMerge() { t := s.T() tests := []struct {