From 21e6574a5166984b5a95cfdd4f2dfef9b53ed26e Mon Sep 17 00:00:00 2001 From: Andy Arthur Date: Thu, 11 Jun 2020 11:09:43 -0500 Subject: [PATCH] refactor --- go/cmd/dolt/commands/query_diff.go | 96 ++++++------------- .../doltcore/diff/querydiff/iter_queue.go | 6 +- .../doltcore/diff/querydiff/query_differ.go | 51 +++++++++- .../doltcore/diff/querydiff/sort_node.go | 10 +- 4 files changed, 87 insertions(+), 76 deletions(-) diff --git a/go/cmd/dolt/commands/query_diff.go b/go/cmd/dolt/commands/query_diff.go index 2684a75684..65db07b062 100644 --- a/go/cmd/dolt/commands/query_diff.go +++ b/go/cmd/dolt/commands/query_diff.go @@ -16,9 +16,6 @@ package commands import ( "context" - "fmt" - "github.com/liquidata-inc/dolt/go/libraries/doltcore/rowconv" - "io" "strings" "github.com/liquidata-inc/go-mysql-server/sql" @@ -30,9 +27,9 @@ import ( "github.com/liquidata-inc/dolt/go/libraries/doltcore/doltdb" "github.com/liquidata-inc/dolt/go/libraries/doltcore/env" "github.com/liquidata-inc/dolt/go/libraries/doltcore/row" + "github.com/liquidata-inc/dolt/go/libraries/doltcore/rowconv" "github.com/liquidata-inc/dolt/go/libraries/doltcore/schema" - "github.com/liquidata-inc/dolt/go/libraries/doltcore/schema/typeinfo" - "github.com/liquidata-inc/dolt/go/libraries/doltcore/sqle" + dsqle "github.com/liquidata-inc/dolt/go/libraries/doltcore/sqle" "github.com/liquidata-inc/dolt/go/libraries/doltcore/table/pipeline" "github.com/liquidata-inc/dolt/go/libraries/doltcore/table/untyped" "github.com/liquidata-inc/dolt/go/libraries/doltcore/table/untyped/fwt" @@ -180,43 +177,35 @@ func maybeResolve(ctx context.Context, dEnv *env.DoltEnv, spec string) (*doltdb. } func validateQueryDiff(ctx context.Context, dEnv *env.DoltEnv, from *doltdb.RootValue, to *doltdb.RootValue, query string) errhand.VerboseError { - sqlCtx, eng, err := makeSqlEngine(ctx, dEnv, to) - if err != nil { - return errhand.BuildDError("Cannot diff query, query is not ordered. Error describing query plan").AddCause(err).Build() - } - - query = fmt.Sprintf("describe %s", query) - _, iter, err := processQuery(sqlCtx, query, eng) - if err != nil { - return errhand.BuildDError("Cannot diff query, query is not ordered. Error describing query plan").AddCause(err).Build() - } - - var qp strings.Builder - for { - r, err := iter.Next() - if err == io.EOF { - break - } else if err != nil { - return errhand.BuildDError("Cannot diff query, query is not ordered. Error describing query plan").AddCause(err).Build() - } - sv, _ := typeinfo.StringDefaultType.ConvertValueToNomsValue(r[0]) - qp.WriteString(fmt.Sprintf("%s\n", string(sv.(types.String)))) - } - - return errhand.BuildDError("Cannot diff query, query is not ordered. Add ORDER BY statement.\nquery plan:\n%s", qp.String()).Build() + //sqlCtx, eng, err := makeSqlEngine(ctx, dEnv, to) + //if err != nil { + // return errhand.BuildDError("Cannot diff query, query is not ordered. Error describing query plan").AddCause(err).Build() + //} + // + //query = fmt.Sprintf("describe %s", query) + //_, iter, err := processQuery(sqlCtx, query, eng) + //if err != nil { + // return errhand.BuildDError("Cannot diff query, query is not ordered. Error describing query plan").AddCause(err).Build() + //} + // + //var qp strings.Builder + //for { + // r, err := iter.Next() + // if err == io.EOF { + // break + // } else if err != nil { + // return errhand.BuildDError("Cannot diff query, query is not ordered. Error describing query plan").AddCause(err).Build() + // } + // sv, _ := typeinfo.StringDefaultType.ConvertValueToNomsValue(r[0]) + // qp.WriteString(fmt.Sprintf("%s\n", string(sv.(types.String)))) + //} + // + //return errhand.BuildDError("Cannot diff query, query is not ordered. Add ORDER BY statement.\nquery plan:\n%s", qp.String()).Build() + return nil } func diffQuery(ctx context.Context, dEnv *env.DoltEnv, fromRoot, toRoot *doltdb.RootValue, query string) errhand.VerboseError { - fromCtx, fromEng, err := makeSqlEngine(ctx, dEnv, fromRoot) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - toCtx, toEng, err := makeSqlEngine(ctx, dEnv, toRoot) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - - qd, err := querydiff.MakeQueryDiffer(fromCtx, toCtx, fromEng.engine, toEng.engine, query) + qd, err := querydiff.MakeQueryDiffer(ctx, dEnv, fromRoot, toRoot, query) if err != nil { return errhand.VerboseErrorFromError(err) } @@ -245,30 +234,8 @@ func diffQuery(ctx context.Context, dEnv *env.DoltEnv, fromRoot, toRoot *doltdb. return errhand.VerboseErrorFromError(p.Wait()) } -const db = "db" - -// todo: we only need the sql.Engine -func makeSqlEngine(ctx context.Context, dEnv *env.DoltEnv, root *doltdb.RootValue) (*sql.Context, *sqlEngine, error) { - mrEnv := env.DoltEnvAsMultiEnv(dEnv) - roots := map[string]*doltdb.RootValue{db: root} - dbs := []sqle.Database{newDatabase(db, dEnv)} - - sqlCtx := sql.NewContext(ctx, - sql.WithSession(sqle.DefaultDoltSession()), - sql.WithIndexRegistry(sql.NewIndexRegistry()), - sql.WithViewRegistry(sql.NewViewRegistry())) - sqlCtx.SetCurrentDatabase(db) - - eng, err := newSqlEngine(sqlCtx, mrEnv, roots, formatTabular, dbs...) - if err != nil { - return nil, nil, err - } - - return sqlCtx, eng, nil -} - func doltSchWithPKFromSqlSchema(sch sql.Schema) schema.Schema { - dSch, _ := sqle.SqlSchemaToDoltResultSchema(sch) + dSch, _ := dsqle.SqlSchemaToDoltResultSchema(sch) // make the first col the PK pk := false newCC, _ := schema.MapColCollection(dSch.GetAllCols(), func(col schema.Column) (column schema.Column, err error) { @@ -290,7 +257,7 @@ func nextQueryDiff(qd *querydiff.QueryDiffer, joiner *rowconv.Joiner) (row.Row, rows := make(map[string]row.Row) if fromRow != nil { sch := joiner.SchemaForName(diff.From) - oldRow, err := sqle.SqlRowToDoltRow(types.Format_Default, fromRow, sch) + oldRow, err := dsqle.SqlRowToDoltRow(types.Format_Default, fromRow, sch) if err != nil { return nil, pipeline.ImmutableProperties{}, err } @@ -299,7 +266,7 @@ func nextQueryDiff(qd *querydiff.QueryDiffer, joiner *rowconv.Joiner) (row.Row, if toRow != nil { sch := joiner.SchemaForName(diff.To) - newRow, err := sqle.SqlRowToDoltRow(types.Format_Default, toRow, sch) + newRow, err := dsqle.SqlRowToDoltRow(types.Format_Default, toRow, sch) if err != nil { return nil, pipeline.ImmutableProperties{}, err } @@ -314,7 +281,6 @@ func nextQueryDiff(qd *querydiff.QueryDiffer, joiner *rowconv.Joiner) (row.Row, return joinedRow, pipeline.ImmutableProperties{}, nil } -// todo: this logic was adapted from commands/diff.go, it could be simplified func buildQueryDiffPipeline(qd *querydiff.QueryDiffer, doltSch schema.Schema, joiner *rowconv.Joiner) (*pipeline.Pipeline, error) { unionSch, ds, verr := createSplitter(doltSch, doltSch, joiner, &diffArgs{diffOutput: TabularDiffOutput}) diff --git a/go/libraries/doltcore/diff/querydiff/iter_queue.go b/go/libraries/doltcore/diff/querydiff/iter_queue.go index 83773bcfda..66786b621e 100644 --- a/go/libraries/doltcore/diff/querydiff/iter_queue.go +++ b/go/libraries/doltcore/diff/querydiff/iter_queue.go @@ -15,9 +15,11 @@ package querydiff import ( - "github.com/liquidata-inc/dolt/go/store/atomicerr" - "github.com/liquidata-inc/go-mysql-server/sql" "io" + + "github.com/liquidata-inc/go-mysql-server/sql" + + "github.com/liquidata-inc/dolt/go/store/atomicerr" ) const ( diff --git a/go/libraries/doltcore/diff/querydiff/query_differ.go b/go/libraries/doltcore/diff/querydiff/query_differ.go index 4f51d9fda0..d804f74388 100644 --- a/go/libraries/doltcore/diff/querydiff/query_differ.go +++ b/go/libraries/doltcore/diff/querydiff/query_differ.go @@ -15,9 +15,9 @@ package querydiff import ( + "context" "errors" "fmt" - "github.com/liquidata-inc/dolt/go/libraries/doltcore/diff" "io" sqle "github.com/liquidata-inc/go-mysql-server" @@ -25,6 +25,9 @@ import ( "github.com/liquidata-inc/go-mysql-server/sql/parse" "github.com/liquidata-inc/go-mysql-server/sql/plan" + "github.com/liquidata-inc/dolt/go/libraries/doltcore/diff" + "github.com/liquidata-inc/dolt/go/libraries/doltcore/doltdb" + "github.com/liquidata-inc/dolt/go/libraries/doltcore/env" dsqle "github.com/liquidata-inc/dolt/go/libraries/doltcore/sqle" ) @@ -36,8 +39,15 @@ type QueryDiffer struct { toIter sql.RowIter } -func MakeQueryDiffer(fromCtx, toCtx *sql.Context, fromEng, toEng *sqle.Engine, query string) (*QueryDiffer, error) { - // todo: make the sql engines here +func MakeQueryDiffer(ctx context.Context, dEnv *env.DoltEnv, fromRoot, toRoot *doltdb.RootValue, query string) (*QueryDiffer, error) { + fromCtx, fromEng, err := makeSqlEngine(ctx, dEnv, fromRoot) + if err != nil { + return nil, err + } + toCtx, toEng, err := makeSqlEngine(ctx, dEnv, toRoot) + if err != nil { + return nil, err + } from, to, err := hackThatPlan(fromCtx, toCtx, fromEng, toEng, query) if err != nil { @@ -140,7 +150,6 @@ func hackThatPlan(fromCtx *sql.Context, toCtx *sql.Context, fromEng *sqle.Engine } func recurseModifyPlans(fromCtx, toCtx *sql.Context, from, to sql.Node) (modFrom, modTo sql.Node, err error) { - // todo: don't assume plans are equal switch from.(type) { case *plan.Sort: nd, err := newSortNodeDiffer(fromCtx, toCtx, from.(*plan.Sort), to.(*plan.Sort)) @@ -169,3 +178,37 @@ func recurseModifyPlans(fromCtx, toCtx *sql.Context, from, to sql.Node) (modFrom } return modFrom, modTo, nil } + +func makeSqlEngine(ctx context.Context, dEnv *env.DoltEnv, root *doltdb.RootValue) (*sql.Context, *sqle.Engine, error) { + doltSqlDB := dsqle.NewDatabase("db", dEnv.DoltDB, dEnv.RepoState, dEnv.RepoStateWriter()) + + sqlCtx := sql.NewContext(ctx, + sql.WithSession(dsqle.DefaultDoltSession()), + sql.WithIndexRegistry(sql.NewIndexRegistry()), + sql.WithViewRegistry(sql.NewViewRegistry())) + sqlCtx.SetCurrentDatabase("db") + + engine := sqle.NewDefault() + engine.AddDatabase(sql.NewInformationSchemaDatabase(engine.Catalog)) + + dsess := dsqle.DSessFromSess(sqlCtx.Session) + + engine.AddDatabase(doltSqlDB) + + err := dsess.AddDB(sqlCtx, doltSqlDB) + if err != nil { + return nil, nil, err + } + + err = doltSqlDB.SetRoot(sqlCtx, root) + if err != nil { + return nil, nil, err + } + + err = dsqle.RegisterSchemaFragments(sqlCtx, doltSqlDB, root) + if err != nil { + return nil, nil, err + } + + return sqlCtx, engine, nil +} diff --git a/go/libraries/doltcore/diff/querydiff/sort_node.go b/go/libraries/doltcore/diff/querydiff/sort_node.go index c7752ff38d..5c5229add8 100644 --- a/go/libraries/doltcore/diff/querydiff/sort_node.go +++ b/go/libraries/doltcore/diff/querydiff/sort_node.go @@ -211,7 +211,7 @@ func (nd *sortNodeDiffer) FromNode() sql.Node { return sqlNodeWrapper{ Node: nd.fromNode, iter: rowIterWrapper{ - next: func() (row sql.Row, err error) { + next: func() (row sql.Row, err error) { return nd.nextFromRow() }, close: func() error { @@ -225,7 +225,7 @@ func (nd *sortNodeDiffer) ToNode() sql.Node { return sqlNodeWrapper{ Node: nd.toNode, iter: rowIterWrapper{ - next: func() (row sql.Row, err error) { + next: func() (row sql.Row, err error) { return nd.nextToRow() }, close: func() error { @@ -242,11 +242,11 @@ func (nd *sortNodeDiffer) Close() error { } // RowIter implements the Node interface. -func (nd *sortNodeDiffer) RowIter(ctx *sql.Context) (sql.RowIter, error) { - panic("RowIter() cannot be called on NodeDiffer, use fromNode() and ToIter()") +func (nd *sortNodeDiffer) RowIter(_ *sql.Context) (sql.RowIter, error) { + panic("RowIter() cannot be called on NodeDiffer, use FromNode() and ToNode()") } // WithChildren implements the Node interface. -func (nd *sortNodeDiffer) WithChildren(children ...sql.Node) (sql.Node, error) { +func (nd *sortNodeDiffer) WithChildren(_ ...sql.Node) (sql.Node, error) { panic("unimplemented") }