Skip to content

Commit

Permalink
Handle single sharded keyspaces for analysis
Browse files Browse the repository at this point in the history
This fixes the problem where we would throw an error for a query on a
single unsharded keyspace, even though the limitation only exists for
sharded queries.

The issue here was that the previous `singleUnshardedKeyspace` field was
really the field indicating if we could short circuit analysis.
`schemadiff` doesn't want to short cut full analysis though, since it
does want to know table & column dependencies etc. which would not be
computed if analysis can be shortcutted.

Since the `singleUnshardedKeyspace` field was overloaded and really was
"can we shortcut" (either because of being in a single sharded keyspace
or because of non-strict analysis being requested), we couldn't use that
field as a fix to guard the error thrown.

Instead, we now have a proper explicit field `canShortcut` that we use
to handle existing shortcut logic.

`singleUnshardedKeyspace` still exists, but now means what it actually
implies, which is if this query runs against a single unsharded
keyspace, irrespective of whether we can shortcut or not and is
correctly set when doing full analysis.

With those changes, we can use `singleUnshardedKeyspace` to guard the
error throw for unsupported sharded queries and `schemadiff` works
properly as well.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
  • Loading branch information
dbussink committed Jun 5, 2024
1 parent 2df3545 commit 3926bec
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 9 deletions.
12 changes: 11 additions & 1 deletion go/vt/schemadiff/schema_diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,16 @@ func TestSchemaDiff(t *testing.T) {
entityOrder: []string{"v2"},
instantCapability: InstantDDLCapabilityIrrelevant,
},
{
name: "add view with over",
toQueries: append(
createQueries,
"create view v2 as SELECT *, ROW_NUMBER() OVER(PARTITION BY info) AS row_num1, ROW_NUMBER() OVER(PARTITION BY info ORDER BY id) AS row_num2 FROM t1;\n",
),
expectDiffs: 1,
entityOrder: []string{"v2"},
instantCapability: InstantDDLCapabilityIrrelevant,
},
{
name: "add view, alter table",
toQueries: []string{
Expand Down Expand Up @@ -1050,7 +1060,7 @@ func TestSchemaDiff(t *testing.T) {
return
}
if tc.conflictingDiffs > 0 {
assert.Error(t, err)
require.Error(t, err)
impossibleOrderErr, ok := err.(*ImpossibleApplyDiffOrderError)
assert.True(t, ok)
conflictingDiffsStatements := []string{}
Expand Down
13 changes: 8 additions & 5 deletions go/vt/vtgate/semantics/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type analyzer struct {
projErr error
unshardedErr error
warning string
canShortcut bool
singleUnshardedKeyspace bool
fullAnalysis bool
}
Expand Down Expand Up @@ -135,7 +136,7 @@ func (a *analyzer) newSemTable(
comments = commentedStmt.GetParsedComments()
}

if a.singleUnshardedKeyspace {
if a.canShortcut {
return &SemTable{
Tables: a.earlyTables.Tables,
Comments: comments,
Expand Down Expand Up @@ -386,16 +387,18 @@ func (a *analyzer) reAnalyze(statement sqlparser.SQLNode) error {
// canShortCut checks if we are dealing with a single unsharded keyspace and no tables that have managed foreign keys
// if so, we can stop the analyzer early
func (a *analyzer) canShortCut(statement sqlparser.Statement) (canShortCut bool) {
if a.fullAnalysis {
ks, _ := singleUnshardedKeyspace(a.earlyTables.Tables)
a.singleUnshardedKeyspace = ks != nil
if !a.singleUnshardedKeyspace {
return false
}
ks, _ := singleUnshardedKeyspace(a.earlyTables.Tables)
if ks == nil {

if a.fullAnalysis {
return false
}

defer func() {
a.singleUnshardedKeyspace = canShortCut
a.canShortcut = canShortCut
}()

if !sqlparser.IsDMLStatement(statement) {
Expand Down
6 changes: 4 additions & 2 deletions go/vt/vtgate/semantics/check_invalid.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@ func (a *analyzer) checkForInvalidConstructs(cursor *sqlparser.Cursor) error {
return vterrors.VT12001("recursive common table expression")
}
case *sqlparser.Insert:
if node.Action == sqlparser.ReplaceAct {
if !a.singleUnshardedKeyspace && node.Action == sqlparser.ReplaceAct {
return ShardedError{Inner: &UnsupportedConstruct{errString: "REPLACE INTO with sharded keyspace"}}
}
case *sqlparser.OverClause:
return ShardedError{Inner: &UnsupportedConstruct{errString: "OVER CLAUSE with sharded keyspace"}}
if !a.singleUnshardedKeyspace {
return ShardedError{Inner: &UnsupportedConstruct{errString: "OVER CLAUSE with sharded keyspace"}}
}
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/semantics/semantic_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ func singleUnshardedKeyspace(tableInfos []TableInfo) (ks *vindexes.Keyspace, tab
return ks, tables
}

// SingleUnshardedKeyspace returns the single keyspace if all tables in the query are in the same keyspace
// SingleKeyspace returns the single keyspace if all tables in the query are in the same keyspace
func (st *SemTable) SingleKeyspace() (ks *vindexes.Keyspace) {
validKS := func(this *vindexes.Keyspace) bool {
if this == nil {
Expand Down

0 comments on commit 3926bec

Please sign in to comment.