Skip to content

Commit

Permalink
changes as per review
Browse files Browse the repository at this point in the history
  • Loading branch information
priyanshi-yb committed Feb 3, 2023
1 parent 381cab3 commit 9998417
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
CREATE INDEX film_fulltext_idx ON public.film USING gist (fulltext);

CREATE INDEX idx_actor_last_name ON public.actor USING btree (last_name);
CREATE INDEX idx_name1 ON table_name USING spgist(col1);
CREATE INDEX idx_name2 ON table_name USING rtree(col1);
CREATE INDEX idx_name1 ON table_name USING spgist (col1);
CREATE INDEX idx_name2 ON table_name USING rtree (col1);

--gin index
CREATE INDEX idx_name3 ON schema_name.table_name USING gin(col1,col2,col3);
CREATE INDEX idx_name3 ON schema_name.table_name USING gin (col1,col2,col3);

--dropping multiple objects
DROP INDEX idx1,idx2,idx3;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ CREATE TABLE xyz PARTITION OF abc;
ALTER TABLE xyz add PRIMARY KEY pk(id);

--foreign table issues
CREATE FOREIGN TABLE tbl_p(id int PRIMARY KEY);
CREATE FOREIGN TABLE tbl_f(fid int, pid int FOREIGN KEY REFERENCES tbl_p(id));
CREATE FOREIGN TABLE tbl_p(
id int PRIMARY KEY
);
CREATE FOREIGN TABLE tbl_f(
fid int,
pid int FOREIGN KEY REFERENCES tbl_p(id)
);

14 changes: 7 additions & 7 deletions migtests/tests/analyze-schema/expected_issues.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,23 @@
"objectType": "INDEX",
"objectName": "idx_name1",
"reason": "index method 'spgist' not supported yet.",
"sqlStatement": "CREATE INDEX idx_name1 ON table_name USING spgist(col1);",
"sqlStatement": "CREATE INDEX idx_name1 ON table_name USING spgist (col1);",
"suggestion": "",
"GH": "https://github.com/YugaByte/yugabyte-db/issues/1337"
},
{
"objectType": "INDEX",
"objectName": "idx_name2",
"reason": "index method 'rtree' is superceded by 'gist' which is not supported yet.",
"sqlStatement": "CREATE INDEX idx_name2 ON table_name USING rtree(col1);",
"sqlStatement": "CREATE INDEX idx_name2 ON table_name USING rtree (col1);",
"suggestion": "",
"GH": "https://github.com/YugaByte/yugabyte-db/issues/1337"
},
{
"objectType": "INDEX",
"objectName": "idx_name3",
"reason": "Schema contains gin index on multi column which is not supported.",
"sqlStatement": "CREATE INDEX idx_name3 ON schema_name.table_name USING gin(col1,col2,col3);",
"sqlStatement": "CREATE INDEX idx_name3 ON schema_name.table_name USING gin (col1,col2,col3);",
"suggestion": "",
"GH": "https://github.com/yugabyte/yugabyte-db/issues/7850"
},
Expand Down Expand Up @@ -201,7 +201,7 @@
},
{
"objectType": "TABLE",
"objectName": "my_table",
"objectName": "e",
"reason": "REINDEX is not supported.",
"sqlStatement": "REINDEX TABLE my_table;",
"suggestion": "",
Expand Down Expand Up @@ -305,7 +305,7 @@
},
{
"objectType": "TABLE",
"objectName": "IF EXISTS ",
"objectName": "test_2",
"reason": "ALTER TABLE SET WITH OIDS not supported yet.",
"sqlStatement": "ALTER TABLE IF EXISTS test_2 SET WITH OIDS;",
"suggestion": "",
Expand Down Expand Up @@ -379,15 +379,15 @@
"objectType": "TABLE",
"objectName": "tbl_p",
"reason": "Primary key constraints are not supported on foreign tables.",
"sqlStatement": "CREATE FOREIGN TABLE tbl_p(id int PRIMARY KEY);",
"sqlStatement": "CREATE FOREIGN TABLE tbl_p(\n\tid int PRIMARY KEY\n);",
"suggestion": "",
"GH": "https://github.com/yugabyte/yugabyte-db/issues/10698"
},
{
"objectType": "TABLE",
"objectName": "tbl_f",
"reason": "Foreign key constraints are not supported on foreign tables.",
"sqlStatement": "CREATE FOREIGN TABLE tbl_f(fid int, pid int FOREIGN KEY REFERENCES tbl_p(id));",
"sqlStatement": "CREATE FOREIGN TABLE tbl_f(\n\tfid int, \n\tpid int FOREIGN KEY REFERENCES tbl_p(id)\n);",
"suggestion": "",
"GH": "https://github.com/yugabyte/yugabyte-db/issues/10699"
},
Expand Down
69 changes: 44 additions & 25 deletions yb-voyager/cmd/analyzeSchema.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,33 @@ type sqlInfo struct {

var (
anything = `.*`
ws = `[\s\n\t]*`
ws = `[\s\n\t]+`
multiWs = `[\s\n\t]*`
ident = `[a-zA-Z0-9_."]+`
ifExists = opt("IF EXISTS")
ifNotExists = opt("IF NOT EXISTS")
commaSeperatedStrings = `[^,]+(?:,[^,]+){0,}`
commaSeperatedStringsMoreThan1 = `[^,]+(?:,[^,]+){1,}`
normalIdent = `[a-zA-Z0-9_]+`
unqualifiedIdent = `[a-zA-Z0-9_]+`
)

func cat(tokens ...string) string {
return strings.Join(tokens, ws)
// join tokens with condition on specific tokens
str := ""
for idx, token := range tokens {
if idx == len(tokens)-1 {
str += token
} else if token == anything || token[len(token)-1] == '?' {
str += token + multiWs
} else {
str += token + ws
}
}
return str
}

func opt(tokens ...string) string {
return fmt.Sprintf("(%s%s)?", cat(tokens...), ws)
return fmt.Sprintf("(%s)?", strings.Join(tokens, ws))
}

func capture(str string) string {
Expand All @@ -79,6 +91,14 @@ func re(tokens ...string) *regexp.Regexp {
return regexp.MustCompile(s)
}

func parenth(s string) string {
return `\(` + s + `\)`
}

func addMultiWs(s string) string {
return multiWs + s
}

var (
outputFormat string
sourceObjList []string
Expand All @@ -92,11 +112,11 @@ var (
//TODO: optional but replace every possible space or new line char with [\s\n]+ in all regexs
createConvRegex = re("CREATE", opt("DEFAULT"), "CONVERSION", capture(ident))
alterConvRegex = re("ALTER", "CONVERSION", capture(ident))
gistRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING GIST")
brinRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING brin")
spgistRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING spgist")
rtreeRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING rtree")
ginRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING GIN", capture(commaSeperatedStrings))
gistRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING", "GIST")
brinRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING", "brin")
spgistRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING", "spgist")
rtreeRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING", "rtree")
ginRegex = re("CREATE", "INDEX", ifNotExists, capture(ident), "ON", capture(ident), anything, "USING", "GIN", capture(commaSeperatedStrings))
viewWithCheckRegex = re("VIEW", capture(ident), anything, "WITH", "CHECK", "OPTION")
rangeRegex = re("PRECEDING", "and", anything, ":float")
fetchRegex = re("FETCH", anything, "FROM")
Expand All @@ -114,14 +134,14 @@ var (
constrTrgRegex = re("CREATE", "CONSTRAINT", "TRIGGER", capture(ident))
currentOfRegex = re("WHERE", "CURRENT", "OF")
amRegex = re("CREATE", "ACCESS", "METHOD", capture(ident))
idxConcRegex = re("REINDEX ", anything, " ", capture(ident))
storedRegex = re(capture(normalIdent), capture(normalIdent), "GENERATED", "ALWAYS", anything, "STORED")
partitionColumnsRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), `\(`+capture(commaSeperatedStrings)+`\) PARTITION BY`, capture("[A-Za-z]+"), `\(`, capture(commaSeperatedStrings), `\)`)
idxConcRegex = re("REINDEX", anything, capture(ident))
storedRegex = re(capture(unqualifiedIdent), capture(unqualifiedIdent), "GENERATED", "ALWAYS", anything, "STORED")
partitionColumnsRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), parenth(capture(commaSeperatedStrings)), "PARTITION BY", capture("[A-Za-z]+"), parenth(capture(commaSeperatedStrings)))
likeAllRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), anything, "LIKE", anything, "INCLUDING ALL")
likeRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), anything, `\(`, "LIKE")
inheritRegex = re("CREATE", opt(capture(normalIdent)), "TABLE", ifNotExists, capture(ident), anything, "INHERITS", "[ |(]")
likeRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), anything, `\(LIKE`)
inheritRegex = re("CREATE", opt(capture(unqualifiedIdent)), "TABLE", ifNotExists, capture(ident), anything, "INHERITS", "[ |(]")
withOidsRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), anything, "WITH", anything, "OIDS")
intvlRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), anything, "interval PRIMARY")
intvlRegex = re("CREATE", "TABLE", ifNotExists, capture(ident)+`\(`, anything, "interval", "PRIMARY")
//super user role required, language c is errored as unsafe
cLangRegex = re("CREATE", opt("OR REPLACE"), "FUNCTION", capture(ident), anything, "language c")

Expand All @@ -133,17 +153,17 @@ var (
alterColumnStorageRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), anything, "ALTER", "COLUMN", capture(ident), anything, "SET STORAGE")
alterColumnSetAttributesRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), anything, "ALTER", "COLUMN", capture(ident), anything, "SET", `\(`)
alterColumnResetAttributesRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), anything, "ALTER", "COLUMN", capture(ident), anything, "RESET")
alterConstrRegex = re("ALTER", opt(capture(normalIdent)), ifExists, "TABLE", capture(ident), anything, "ALTER", "CONSTRAINT")
setOidsRegex = re("ALTER", opt(capture(normalIdent)), "TABLE", ifExists, capture(ident), anything, "SET WITH OIDS")
alterConstrRegex = re("ALTER", opt(capture(unqualifiedIdent)), ifExists, "TABLE", capture(ident), anything, "ALTER", "CONSTRAINT")
setOidsRegex = re("ALTER", opt(capture(unqualifiedIdent)), "TABLE", ifExists, capture(ident), anything, "SET WITH OIDS")
clusterRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), anything, "CLUSTER")
withoutClusterRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), anything, "SET WITHOUT CLUSTER")
alterSetRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), "SET")
alterIdxRegex = re("ALTER", "INDEX", capture(ident), "SET")
alterResetRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), "RESET")
alterOptionsRegex = re("ALTER", opt(capture(normalIdent)), "TABLE", ifExists, capture(ident), "OPTIONS")
alterInhRegex = re("ALTER", opt(capture(normalIdent)), "TABLE", ifExists, capture(ident), "INHERIT")
valConstrRegex = re("ALTER", opt(capture(normalIdent)), "TABLE", ifExists, capture(ident), "VALIDATE CONSTRAINT")
deferRegex = re("ALTER", opt(capture(normalIdent)), "TABLE", ifExists, capture(ident), anything, "UNIQUE", anything, "deferrable")
alterOptionsRegex = re("ALTER", opt(capture(unqualifiedIdent)), "TABLE", ifExists, capture(ident), "OPTIONS")
alterInhRegex = re("ALTER", opt(capture(unqualifiedIdent)), "TABLE", ifExists, capture(ident), "INHERIT")
valConstrRegex = re("ALTER", opt(capture(unqualifiedIdent)), "TABLE", ifExists, capture(ident), "VALIDATE CONSTRAINT")
deferRegex = re("ALTER", opt(capture(unqualifiedIdent)), "TABLE", ifExists, capture(ident), anything, "UNIQUE", anything, "deferrable")
alterViewRegex = re("ALTER", "VIEW", capture(ident))
dropAttrRegex = re("ALTER", "TYPE", capture(ident), "DROP ATTRIBUTE")
alterTypeRegex = re("ALTER", "TYPE", capture(ident))
Expand All @@ -152,8 +172,8 @@ var (
// table partition. partitioned table is the key in tblParts map
tblPartitionRegex = re("CREATE", "TABLE", ifNotExists, capture(ident), anything, "PARTITION", "OF", capture(ident))
addPrimaryRegex = re("ALTER", "TABLE", opt("ONLY"), ifExists, capture(ident), anything, "ADD PRIMARY KEY")
primRegex = re("CREATE", "FOREIGN", "TABLE", capture(ident), anything, "PRIMARY KEY")
foreignKeyRegex = re("CREATE", "FOREIGN", "TABLE", capture(ident), anything, "REFERENCES")
primRegex = re("CREATE", "FOREIGN", "TABLE", capture(ident)+`\(`, anything, "PRIMARY KEY")
foreignKeyRegex = re("CREATE", "FOREIGN", "TABLE", capture(ident)+`\(`, anything, "REFERENCES")

// unsupported SQLs exported by ora2pg
compoundTrigRegex = re("CREATE", opt("OR REPLACE"), "TRIGGER", capture(ident), anything, "COMPOUND", anything)
Expand Down Expand Up @@ -367,7 +387,6 @@ func checkDDL(sqlInfoArr []sqlInfo, fpath string) {
reportCase(fpath, "CREATE ACCESS METHOD is not supported.",
"https://github.com/yugabyte/yugabyte-db/issues/10693", "", "ACCESS METHOD", am[1], sqlInfo.formattedStmt)
} else if tbl := idxConcRegex.FindStringSubmatch(sqlInfo.stmt); tbl != nil {
fmt.Println(idxConcRegex, sqlInfo.stmt, len(tbl), tbl[1])
reportCase(fpath, "REINDEX is not supported.",
"https://github.com/yugabyte/yugabyte-db/issues/10267", "", "TABLE", tbl[1], sqlInfo.formattedStmt)
} else if col := storedRegex.FindStringSubmatch(sqlInfo.stmt); col != nil {
Expand Down Expand Up @@ -432,7 +451,7 @@ func checkDDL(sqlInfoArr []sqlInfo, fpath string) {
"https://github.com/YugaByte/yugabyte-db/issues/1124", "", "TABLE", tbl[3], sqlInfo.formattedStmt)
} else if tbl := setOidsRegex.FindStringSubmatch(sqlInfo.stmt); tbl != nil {
reportCase(fpath, "ALTER TABLE SET WITH OIDS not supported yet.",
"https://github.com/YugaByte/yugabyte-db/issues/1124", "", "TABLE", tbl[3], sqlInfo.formattedStmt)
"https://github.com/YugaByte/yugabyte-db/issues/1124", "", "TABLE", tbl[4], sqlInfo.formattedStmt)
} else if tbl := withoutClusterRegex.FindStringSubmatch(sqlInfo.stmt); tbl != nil {
reportCase(fpath, "ALTER TABLE SET WITHOUT CLUSTER not supported yet.",
"https://github.com/YugaByte/yugabyte-db/issues/1124", "", "TABLE", tbl[2], sqlInfo.formattedStmt)
Expand Down

0 comments on commit 9998417

Please sign in to comment.