vitessio / vitess

Vitess is a database clustering system for horizontal scaling of MySQL.
http://vitess.io
Apache License 2.0
18.53k stars 2.09k forks source link

Bug Report: primaryVindexesDiffer is returning the wrong value #16565

Open CAEL0 opened 2 months ago

CAEL0 commented 2 months ago

Overview of the Issue

In go/vt/wrangler/materializer.go and go/vt/vtctl/workflow/materializer.go, the function primaryVindexesDiffer returns true if neither source nor target have any vindexes.

// primaryVindexesDiffer returns true if, for any tables defined in the provided
// materialize settings, the source and target vschema definitions for those
// tables have different primary vindexes.
//
// The result of this function is used to determine whether to apply a source
// shard selection optimization in MoveTables.
func primaryVindexesDiffer(ms *vtctldatapb.MaterializeSettings, source, target *vschemapb.Keyspace) bool {

    ...

        // If neither source nor target have any vindexes, treat the answer to
        // the question as trivially false.
        if len(sColumnVindexes) == 0 && len(tColumnVindexes) == 0 {
            return true
        }

    ...

But it should return false as written in the comment.

Reproduction Steps

N/A

Binary Version

N/A

Operating System and Environment details

N/A

Log Fragments

No response

mattlord commented 2 months ago

Thanks, @CAEL0 ! Due to the way this feature works, true or false does not matter if the source and target are unsharded as there's no optimization to make. So while this doesn't have a practical impact, obviously this return value is wrong and we should fix it. 🙂