From 244d6d8b8c353021d83175d4c42a706a26353e17 Mon Sep 17 00:00:00 2001 From: tbantle22 Date: Thu, 23 Feb 2023 20:29:24 +0000 Subject: [PATCH 1/3] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/go.sum | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/go/go.sum b/go/go.sum index cf4a469c14..a53938ec5b 100644 --- a/go/go.sum +++ b/go/go.sum @@ -638,6 +638,7 @@ github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/yusufpapurcu/wmi v1.2.2 h1:KBNDSne4vP5mbSWnJbO+51IMOXJB67QiYCSBrubbPRg= github.com/yusufpapurcu/wmi v1.2.2/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= github.com/zeebo/assert v1.1.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= @@ -744,6 +745,7 @@ golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzB golang.org/x/mod v0.1.1-0.20191107180719-034126e5016b/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA= golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -778,7 +780,6 @@ golang.org/x/net v0.0.0-20200501053045-e0ff5e5a1de5/go.mod h1:qpuaurCH72eLCgpAm/ golang.org/x/net v0.0.0-20200506145744-7e3656a0809f/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200513185701-a91f0712d120/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= -golang.org/x/net v0.0.0-20200602114024-627f9648deb9/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= @@ -789,6 +790,7 @@ golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLd golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= +golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g= golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -810,6 +812,7 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -853,7 +856,6 @@ golang.org/x/sys v0.0.0-20200511232937-7e40ca221e25/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200523222454-059865788121/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200615200032-f1bc736245b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200620081246-981b61492c35/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200803210538-64077c9b5642/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200828194041-157a740278f4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -876,6 +878,8 @@ golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220111092808-5a964db01320/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -892,6 +896,7 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -954,6 +959,7 @@ golang.org/x/tools v0.0.0-20200915173823-2db8f0ff891c/go.mod h1:z6u4i615ZeAfBE4X golang.org/x/tools v0.0.0-20200918232735-d647fc253266/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.3.0 h1:SrNbZl6ECOS1qFzgTdQfWXZM9XBkiA6tkFrH9YSTPHM= golang.org/x/tools v0.3.0/go.mod h1:/rWhSS2+zyEVwoJf8YAX6L2f0ntZ7Kn/mGgAWcipA5k= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From 89a145b95a2c8c6a421c61f906c87cf15b18ba9d Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Thu, 23 Feb 2023 13:39:53 -0800 Subject: [PATCH 2/3] Fix bats --- go/cmd/dolt/commands/diff.go | 110 +++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 51 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index a9ae3c2867..433d5c1a04 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -719,14 +719,11 @@ func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string return ddlStatements, nil } -func getRowDiffIter( - ctx context.Context, +func getDataHasChanged(ctx context.Context, se *engine.SqlEngine, td diff.TableDelta, dArgs *diffArgs, - where string, - limit int, -) (*sql.Context, sql.Schema, sql.RowIter, string, errhand.VerboseError) { +) (bool, errhand.VerboseError) { diffable := schema.ArePrimaryKeySetsDiffable(td.Format(), td.FromSch, td.ToSch) canSqlDiff := !(td.ToSch == nil || (td.FromSch != nil && !schema.SchemasAreEqual(td.FromSch, td.ToSch))) @@ -734,55 +731,33 @@ func getRowDiffIter( if !diffable { // TODO: this messes up some structured output if the user didn't redirect it cli.PrintErrf("Primary key sets differ between revisions for table %s, skipping data diff\n", td.ToName) - return nil, nil, nil, "", nil + return false, nil } else if dArgs.diffOutput == SQLDiffOutput && !canSqlDiff { // TODO: this is overly broad, we can absolutely do better _, _ = fmt.Fprintf(cli.CliErr, "Incompatible schema change, skipping data diff\n") - return nil, nil, nil, "", nil + return false, nil } - // do the data diff tableName := td.CurName() columns := getColumnNamesString(td.FromSch, td.ToSch) - query := fmt.Sprintf("select %s, %s from dolt_diff('%s', '%s', '%s')", columns, "diff_type", dArgs.fromRef, dArgs.toRef, tableName) - - if len(where) > 0 { - query += " where " + where - } - - if limit >= 0 { - query += " limit " + strconv.Itoa(limit) - } + query := fmt.Sprintf("select %s, %s from dolt_diff('%s', '%s', '%s') limit 1", columns, "diff_type", dArgs.fromRef, dArgs.toRef, tableName) sqlCtx, err := engine.NewLocalSqlContext(ctx, se) if err != nil { - return nil, nil, nil, "", errhand.VerboseErrorFromError(err) + return false, errhand.VerboseErrorFromError(err) } - sch, rowIter, err := se.Query(sqlCtx, query) + _, rowIter, err := se.Query(sqlCtx, query) if sql.ErrSyntaxError.Is(err) { - return nil, nil, nil, "", errhand.BuildDError("Failed to parse diff query. Invalid where clause?\nDiff query: %s", query).AddCause(err).Build() + return false, errhand.BuildDError("Failed to parse diff query. Invalid where clause?\nDiff query: %s", query).AddCause(err).Build() } else if err != nil { - return nil, nil, nil, "", errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() - } - - return sqlCtx, sch, rowIter, query, nil -} - -func getDataHasChanged(ctx context.Context, - se *engine.SqlEngine, - td diff.TableDelta, - dArgs *diffArgs, -) (bool, errhand.VerboseError) { - sqlCtx, _, rowIter, _, verr := getRowDiffIter(ctx, se, td, dArgs, "", 1) - if verr != nil { - return false, verr + return false, errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() } defer rowIter.Close(sqlCtx) - _, err := rowIter.Next(sqlCtx) + _, err = rowIter.Next(sqlCtx) if err == io.EOF { return false, nil } else if err != nil { @@ -799,6 +774,9 @@ func diffRows( dArgs *diffArgs, dw diffWriter, ) errhand.VerboseError { + diffable := schema.ArePrimaryKeySetsDiffable(td.Format(), td.FromSch, td.ToSch) + canSqlDiff := !(td.ToSch == nil || (td.FromSch != nil && !schema.SchemasAreEqual(td.FromSch, td.ToSch))) + var toSch, fromSch sql.Schema if td.FromSch != nil { pkSch, err := sqlutil.FromDoltSchema(td.FromName, td.FromSch) @@ -807,7 +785,6 @@ func diffRows( } fromSch = pkSch.Schema } - if td.ToSch != nil { pkSch, err := sqlutil.FromDoltSchema(td.ToName, td.ToSch) if err != nil { @@ -815,15 +792,32 @@ func diffRows( } toSch = pkSch.Schema } - unionSch := unionSchemas(fromSch, toSch) - // We always instantiate a RowWriter in case the diffWriter needs it to close off any work from schema output rowWriter, err := dw.RowWriter(ctx, td, unionSch) if err != nil { return errhand.VerboseErrorFromError(err) } + // can't diff + if !diffable { + // TODO: this messes up some structured output if the user didn't redirect it + cli.PrintErrf("Primary key sets differ between revisions for table %s, skipping data diff\n", td.ToName) + err := rowWriter.Close(ctx) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + return nil + } else if dArgs.diffOutput == SQLDiffOutput && !canSqlDiff { + // TODO: this is overly broad, we can absolutely do better + _, _ = fmt.Fprintf(cli.CliErr, "Incompatible schema change, skipping data diff\n") + err := rowWriter.Close(ctx) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + return nil + } + // no data diff requested if dArgs.diffParts&DataOnlyDiff == 0 { err := rowWriter.Close(ctx) @@ -833,25 +827,43 @@ func diffRows( return nil } - sqlCtx, sch, rowIter, query, verr := getRowDiffIter(ctx, se, td, dArgs, dArgs.where, dArgs.limit) - if verr != nil { - err := rowWriter.Close(ctx) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - return verr + // do the data diff + tableName := td.ToName + if len(tableName) == 0 { + tableName = td.FromName + } + + columns := getColumnNamesString(td.FromSch, td.ToSch) + query := fmt.Sprintf("select %s, %s from dolt_diff('%s', '%s', '%s')", columns, "diff_type", dArgs.fromRef, dArgs.toRef, tableName) + + if len(dArgs.where) > 0 { + query += " where " + dArgs.where + } + + if dArgs.limit >= 0 { + query += " limit " + strconv.Itoa(dArgs.limit) + } + + sqlCtx, err := engine.NewLocalSqlContext(ctx, se) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + + sch, rowIter, err := se.Query(sqlCtx, query) + if sql.ErrSyntaxError.Is(err) { + return errhand.BuildDError("Failed to parse diff query. Invalid where clause?\nDiff query: %s", query).AddCause(err).Build() + } else if err != nil { + return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() } defer rowIter.Close(sqlCtx) defer rowWriter.Close(ctx) - var modifiedColNames map[string]bool if dArgs.skinny { modifiedColNames, err = getModifiedCols(sqlCtx, rowIter, unionSch, sch) if err != nil { return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() } - // instantiate a new schema that only contains the columns with changes var filteredUnionSch sql.Schema for _, s := range unionSch { @@ -861,14 +873,12 @@ func diffRows( } } } - // instantiate a new RowWriter with the new schema that only contains the columns with changes rowWriter, err = dw.RowWriter(ctx, td, filteredUnionSch) if err != nil { return errhand.VerboseErrorFromError(err) } defer rowWriter.Close(ctx) - // reset the row iterator err = rowIter.Close(sqlCtx) if err != nil { @@ -882,12 +892,10 @@ func diffRows( return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() } } - err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter, modifiedColNames, dArgs) if err != nil { return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() } - return nil } From 2f818e903cb47d578e3929d68018a49d82e7e715 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Fri, 24 Feb 2023 10:27:37 -0800 Subject: [PATCH 3/3] Use processTableDelta logic in td.GetSummary --- go/cmd/dolt/commands/diff.go | 61 +------------- go/libraries/doltcore/diff/table_deltas.go | 84 ++++++++++++++++--- .../sqle/dtables/unscoped_diff_table.go | 84 +++---------------- 3 files changed, 89 insertions(+), 140 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 433d5c1a04..660e3296b3 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -479,7 +479,7 @@ func maybeResolve(ctx context.Context, dEnv *env.DoltEnv, spec string) (*doltdb. return root, true } -func printDiffSummary(ctx context.Context, tds []diff.TableDelta, engine *engine.SqlEngine, dArgs *diffArgs) errhand.VerboseError { +func printDiffSummary(ctx context.Context, tds []diff.TableDelta, ddb *doltdb.DoltDB, dArgs *diffArgs) errhand.VerboseError { sqlSch := sql.Schema{ &sql.Column{Name: "Table name", Type: types.Text, Nullable: false}, &sql.Column{Name: "Diff Type", Type: types.Text, Nullable: false}, @@ -500,17 +500,12 @@ func printDiffSummary(ctx context.Context, tds []diff.TableDelta, engine *engine return errhand.BuildDError("error: both tables in tableDelta are nil").Build() } - dataChanged, verr := getDataHasChanged(ctx, engine, td, dArgs) - if verr != nil { - return verr - } - - summ, err := td.GetSummary(ctx, dataChanged) + summ, err := td.GetSummary(ctx) if err != nil { return errhand.BuildDError("could not get table delta summary").AddCause(err).Build() } - err = wr.WriteSqlRow(ctx, sql.Row{td.CurName(), summ.DiffType, dataChanged, summ.HasSchemaChanges}) + err = wr.WriteSqlRow(ctx, sql.Row{td.CurName(), summ.DiffType, summ.DataChange, summ.SchemaChange}) if err != nil { return errhand.BuildDError("could not write table delta summary").AddCause(err).Build() } @@ -537,7 +532,7 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err }) if dArgs.diffParts&Summary != 0 { - return printDiffSummary(ctx, tableDeltas, engine, dArgs) + return printDiffSummary(ctx, tableDeltas, dEnv.DoltDB, dArgs) } dw, err := newDiffWriter(dArgs.diffOutput) @@ -719,54 +714,6 @@ func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string return ddlStatements, nil } -func getDataHasChanged(ctx context.Context, - se *engine.SqlEngine, - td diff.TableDelta, - dArgs *diffArgs, -) (bool, errhand.VerboseError) { - diffable := schema.ArePrimaryKeySetsDiffable(td.Format(), td.FromSch, td.ToSch) - canSqlDiff := !(td.ToSch == nil || (td.FromSch != nil && !schema.SchemasAreEqual(td.FromSch, td.ToSch))) - - // can't diff - if !diffable { - // TODO: this messes up some structured output if the user didn't redirect it - cli.PrintErrf("Primary key sets differ between revisions for table %s, skipping data diff\n", td.ToName) - return false, nil - } else if dArgs.diffOutput == SQLDiffOutput && !canSqlDiff { - // TODO: this is overly broad, we can absolutely do better - _, _ = fmt.Fprintf(cli.CliErr, "Incompatible schema change, skipping data diff\n") - return false, nil - } - - tableName := td.CurName() - - columns := getColumnNamesString(td.FromSch, td.ToSch) - query := fmt.Sprintf("select %s, %s from dolt_diff('%s', '%s', '%s') limit 1", columns, "diff_type", dArgs.fromRef, dArgs.toRef, tableName) - - sqlCtx, err := engine.NewLocalSqlContext(ctx, se) - if err != nil { - return false, errhand.VerboseErrorFromError(err) - } - - _, rowIter, err := se.Query(sqlCtx, query) - if sql.ErrSyntaxError.Is(err) { - return false, errhand.BuildDError("Failed to parse diff query. Invalid where clause?\nDiff query: %s", query).AddCause(err).Build() - } else if err != nil { - return false, errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() - } - - defer rowIter.Close(sqlCtx) - - _, err = rowIter.Next(sqlCtx) - if err == io.EOF { - return false, nil - } else if err != nil { - return false, errhand.VerboseErrorFromError(err) - } - - return true, nil -} - func diffRows( ctx context.Context, se *engine.SqlEngine, diff --git a/go/libraries/doltcore/diff/table_deltas.go b/go/libraries/doltcore/diff/table_deltas.go index 08861dd576..41412836a9 100644 --- a/go/libraries/doltcore/diff/table_deltas.go +++ b/go/libraries/doltcore/diff/table_deltas.go @@ -58,10 +58,10 @@ type TableDelta struct { } type TableDeltaSummary struct { - DiffType string - HasDataChanges bool - HasSchemaChanges bool - TableName string + DiffType string + DataChange bool + SchemaChange bool + TableName string } // GetStagedUnstagedTableDeltas represents staged and unstaged changes as TableDelta slices. @@ -287,13 +287,16 @@ func (td TableDelta) IsRename() bool { return td.FromName != td.ToName } -func (td TableDelta) Type() string { +func (td TableDelta) TypeString() string { if td.IsAdd() { return "added" } if td.IsDrop() { return "dropped" } + if td.IsRename() { + return "renamed" + } return "modified" } @@ -404,17 +407,78 @@ func (td TableDelta) IsKeyless(ctx context.Context) (bool, error) { } } +// isTableDataEmpty return true if the table does not contain any data +func isTableDataEmpty(ctx context.Context, table *doltdb.Table) (bool, error) { + rowData, err := table.GetRowData(ctx) + if err != nil { + return false, err + } + + return rowData.Empty() +} + // GetSummary returns a summary of the table delta. -func (td TableDelta) GetSummary(ctx context.Context, dataChanged bool) (*TableDeltaSummary, error) { +func (td TableDelta) GetSummary(ctx context.Context) (*TableDeltaSummary, error) { + // Dropping a table is always a schema change, and also a data change if the table contained data + if td.IsDrop() { + isEmpty, err := isTableDataEmpty(ctx, td.FromTable) + if err != nil { + return nil, err + } + + return &TableDeltaSummary{ + TableName: td.FromName, + DataChange: !isEmpty, + SchemaChange: true, + DiffType: "dropped", + }, nil + } + + // Renaming a table is always a schema change, and also a data change if the table data differs + if td.IsRename() { + dataChanged, err := td.HasHashChanged() + if err != nil { + return nil, err + } + + return &TableDeltaSummary{ + TableName: td.ToName, + DataChange: dataChanged, + SchemaChange: true, + DiffType: "renamed", + }, nil + } + + // Creating a table is always a schema change, and also a data change if data was inserted + if td.IsAdd() { + isEmpty, err := isTableDataEmpty(ctx, td.ToTable) + if err != nil { + return nil, err + } + + return &TableDeltaSummary{ + TableName: td.ToName, + DataChange: !isEmpty, + SchemaChange: true, + DiffType: "added", + }, nil + } + + dataChanged, err := td.HasHashChanged() + if err != nil { + return nil, err + } + schemaChanged, err := td.HasSchemaChanged(ctx) if err != nil { return nil, err } + return &TableDeltaSummary{ - HasSchemaChanges: schemaChanged, - HasDataChanges: dataChanged, - DiffType: td.Type(), - TableName: td.CurName(), + TableName: td.ToName, + DataChange: dataChanged, + SchemaChange: schemaChanged, + DiffType: "modified", }, nil } diff --git a/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go b/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go index 53d58d26d0..6e3097228e 100644 --- a/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go +++ b/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go @@ -241,20 +241,20 @@ func (d *doltDiffWorkingSetRowItr) Next(ctx *sql.Context) (sql.Row, error) { return nil, io.EOF } - change, err := processTableDelta(ctx, tableDelta) + change, err := tableDelta.GetSummary(ctx) if err != nil { return nil, err } sqlRow := sql.NewRow( changeSet, - change.tableName, + change.TableName, nil, // committer nil, // email nil, // date nil, // message - change.dataChange, - change.schemaChange, + change.DataChange, + change.SchemaChange, ) return sqlRow, nil @@ -288,7 +288,7 @@ type doltDiffCommitHistoryRowItr struct { commits []*doltdb.Commit meta *datas.CommitMeta hash hash.Hash - tableChanges []tableChange + tableChanges []diff.TableDeltaSummary tableChangesIdx int } @@ -358,13 +358,13 @@ func (itr *doltDiffCommitHistoryRowItr) Next(ctx *sql.Context) (sql.Row, error) return sql.NewRow( h.String(), - tableChange.tableName, + tableChange.TableName, meta.Name, meta.Email, meta.Time(), meta.Description, - tableChange.dataChange, - tableChange.schemaChange, + tableChange.DataChange, + tableChange.SchemaChange, ), nil } @@ -399,7 +399,7 @@ func (itr *doltDiffCommitHistoryRowItr) loadTableChanges(ctx context.Context, co // calculateTableChanges calculates the tables that changed in the specified commit, by comparing that // commit with its immediate ancestor commit. -func (itr *doltDiffCommitHistoryRowItr) calculateTableChanges(ctx context.Context, commit *doltdb.Commit) ([]tableChange, error) { +func (itr *doltDiffCommitHistoryRowItr) calculateTableChanges(ctx context.Context, commit *doltdb.Commit) ([]diff.TableDeltaSummary, error) { if len(commit.DatasParents()) == 0 { return nil, nil } @@ -424,9 +424,9 @@ func (itr *doltDiffCommitHistoryRowItr) calculateTableChanges(ctx context.Contex return nil, err } - tableChanges := make([]tableChange, len(deltas)) + tableChanges := make([]diff.TableDeltaSummary, len(deltas)) for i := 0; i < len(deltas); i++ { - change, err := processTableDelta(itr.ctx, deltas[i]) + change, err := deltas[i].GetSummary(itr.ctx) if err != nil { return nil, err } @@ -442,68 +442,6 @@ func (itr *doltDiffCommitHistoryRowItr) calculateTableChanges(ctx context.Contex return tableChanges, nil } -// processTableDelta processes the specified TableDelta to determine what kind of change it was (i.e. table drop, -// table rename, table create, or data update) and returns a tableChange struct representing the change. -func processTableDelta(ctx *sql.Context, delta diff.TableDelta) (*tableChange, error) { - // Dropping a table is always a schema change, and also a data change if the table contained data - if delta.IsDrop() { - isEmpty, err := isTableDataEmpty(ctx, delta.FromTable) - if err != nil { - return nil, err - } - - return &tableChange{ - tableName: delta.FromName, - dataChange: !isEmpty, - schemaChange: true, - }, nil - } - - // Renaming a table is always a schema change, and also a data change if the table data differs - if delta.IsRename() { - dataChanged, err := delta.HasHashChanged() - if err != nil { - return nil, err - } - - return &tableChange{ - tableName: delta.ToName, - dataChange: dataChanged, - schemaChange: true, - }, nil - } - - // Creating a table is always a schema change, and also a data change if data was inserted - if delta.IsAdd() { - isEmpty, err := isTableDataEmpty(ctx, delta.ToTable) - if err != nil { - return nil, err - } - - return &tableChange{ - tableName: delta.ToName, - dataChange: !isEmpty, - schemaChange: true, - }, nil - } - - dataChanged, err := delta.HasHashChanged() - if err != nil { - return nil, err - } - - schemaChanged, err := delta.HasSchemaChanged(ctx) - if err != nil { - return nil, err - } - - return &tableChange{ - tableName: delta.ToName, - dataChange: dataChanged, - schemaChange: schemaChanged, - }, nil -} - // Close closes the iterator. func (itr *doltDiffCommitHistoryRowItr) Close(*sql.Context) error { return nil