Skip to content

Commit

Permalink
add function names in details
Browse files Browse the repository at this point in the history
  • Loading branch information
@priyanshi-yb
priyanshi-yb committed Dec 21, 2024
1 parent 6a96cdd commit 1bb6de4
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 46 deletions.
1 change: 1 addition & 0 deletions yb-voyager/src/query/queryissue/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const (
// Object types
const (
CONSTRAINT_NAME = "ConstraintName"
FUNCTION_NAMES = "FunctionNames"
TABLE_OBJECT_TYPE = "TABLE"
FOREIGN_TABLE_OBJECT_TYPE = "FOREIGN TABLE"
FUNCTION_OBJECT_TYPE = "FUNCTION"
Expand Down
15 changes: 8 additions & 7 deletions yb-voyager/src/query/queryissue/detectors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type FuncCallDetector struct {
// right now it covers Advisory Locks and XML functions
advisoryLocksFuncsDetected mapset.Set[string]
xmlFuncsDetected mapset.Set[string]
anyValAggDetected bool
aggFuncsDetected mapset.Set[string]
loFuncsDetected mapset.Set[string]
}

Expand All @@ -44,6 +44,7 @@ func NewFuncCallDetector(query string) *FuncCallDetector {
query: query,
advisoryLocksFuncsDetected: mapset.NewThreadUnsafeSet[string](),
xmlFuncsDetected: mapset.NewThreadUnsafeSet[string](),
aggFuncsDetected: mapset.NewThreadUnsafeSet[string](),
loFuncsDetected: mapset.NewThreadUnsafeSet[string](),
}
}
Expand All @@ -65,7 +66,7 @@ func (d *FuncCallDetector) Detect(msg protoreflect.Message) error {
}

if unsupportedAggFunctions.ContainsOne(funcName) {
d.anyValAggDetected = true
d.aggFuncsDetected.Add(funcName)
}
if unsupportedLargeObjectFunctions.ContainsOne(funcName) {
d.loFuncsDetected.Add(funcName)
Expand All @@ -82,11 +83,11 @@ func (d *FuncCallDetector) GetIssues() []QueryIssue {
if d.xmlFuncsDetected.Cardinality() > 0 {
issues = append(issues, NewXmlFunctionsIssue(DML_QUERY_OBJECT_TYPE, "", d.query))
}
if d.anyValAggDetected {
issues = append(issues, NewAggregationFunctionIssue(DML_QUERY_OBJECT_TYPE, "", d.query))
if d.aggFuncsDetected.Cardinality() > 0 {
issues = append(issues, NewAggregationFunctionIssue(DML_QUERY_OBJECT_TYPE, "", d.query, d.aggFuncsDetected.ToSlice()))
}
if d.loFuncsDetected.Cardinality() > 0 {
issues = append(issues, NewLOFuntionsIssue(DML_QUERY_OBJECT_TYPE, "", d.query))
issues = append(issues, NewLOFuntionsIssue(DML_QUERY_OBJECT_TYPE, "", d.query, d.loFuncsDetected.ToSlice()))
}
return issues
}
Expand Down Expand Up @@ -224,7 +225,7 @@ func (j *JsonConstructorFuncDetector) Detect(msg protoreflect.Message) error {
func (d *JsonConstructorFuncDetector) GetIssues() []QueryIssue {
var issues []QueryIssue
if d.unsupportedJsonConstructorFunctionsDetected.Cardinality() > 0 {
issues = append(issues, NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", d.query))
issues = append(issues, NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", d.query, d.unsupportedJsonConstructorFunctionsDetected.ToSlice()))
}
return issues
}
Expand Down Expand Up @@ -277,7 +278,7 @@ func (j *JsonQueryFunctionDetector) Detect(msg protoreflect.Message) error {
func (d *JsonQueryFunctionDetector) GetIssues() []QueryIssue {
var issues []QueryIssue
if d.unsupportedJsonQueryFunctionsDetected.Cardinality() > 0 {
issues = append(issues, NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", d.query))
issues = append(issues, NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", d.query, d.unsupportedJsonQueryFunctionsDetected.ToSlice()))
}
return issues
}
1 change: 1 addition & 0 deletions yb-voyager/src/query/queryissue/detectors_ddl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ func (tid *TriggerIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]Quer
obj.GetObjectType(),
trigger.GetObjectName(),
"",
[]string{trigger.FuncName},
))
}

Expand Down
34 changes: 23 additions & 11 deletions yb-voyager/src/query/queryissue/issues_dml.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,40 +62,49 @@ func NewXmlFunctionsIssue(objectType string, objectName string, sqlStatement str
var anyValueAggFunctionIssue = issue.Issue{
Type: AGGREGATE_FUNCTION,
TypeName: AGGREGATION_FUNCTIONS_NAME,
TypeDescription: "",
TypeDescription: "Postgresql 17 features not supported yet in YugabyteDB",
Suggestion: "",
GH: "",
DocsLink: "",
}

func NewAggregationFunctionIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(anyValueAggFunctionIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
func NewAggregationFunctionIssue(objectType string, objectName string, sqlStatement string, funcNames []string) QueryIssue {
details := map[string]interface{}{
FUNCTION_NAMES: funcNames,//TODO USE it later when we start putting these in reports
}
return newQueryIssue(anyValueAggFunctionIssue, objectType, objectName, sqlStatement, details)
}

var jsonConstructorFunctionsIssue = issue.Issue{
Type: JSON_CONSTRUCTOR_FUNCTION,
TypeName: JSON_CONSTRUCTOR_FUNCTION_NAME,
TypeDescription: "",
TypeDescription: "Postgresql 17 features not supported yet in YugabyteDB",
Suggestion: "",
GH: "",
DocsLink: "",
}

func NewJsonConstructorFunctionIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(jsonConstructorFunctionsIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
func NewJsonConstructorFunctionIssue(objectType string, objectName string, sqlStatement string, funcNames []string) QueryIssue {
details := map[string]interface{}{
FUNCTION_NAMES: funcNames,//TODO USE it later when we start putting these in reports
}
return newQueryIssue(jsonConstructorFunctionsIssue, objectType, objectName, sqlStatement, details)
}

var jsonQueryFunctionIssue = issue.Issue{
Type: JSON_QUERY_FUNCTION,
TypeName: JSON_QUERY_FUNCTIONS_NAME,
TypeDescription: "",
TypeDescription: "Postgresql 17 features not supported yet in YugabyteDB",
Suggestion: "",
GH: "",
DocsLink: "",
}

func NewJsonQueryFunctionIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(jsonQueryFunctionIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
func NewJsonQueryFunctionIssue(objectType string, objectName string, sqlStatement string, funcNames []string) QueryIssue {
details := map[string]interface{}{
FUNCTION_NAMES: funcNames, //TODO USE it later when we start putting these in reports
}
return newQueryIssue(jsonQueryFunctionIssue, objectType, objectName, sqlStatement, details)
}

var loFunctionsIssue = issue.Issue{
Expand All @@ -107,6 +116,9 @@ var loFunctionsIssue = issue.Issue{
DocsLink: "", //TODO
}

func NewLOFuntionsIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(loFunctionsIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
func NewLOFuntionsIssue(objectType string, objectName string, sqlStatement string, funcNames []string) QueryIssue {
details := map[string]interface{}{
FUNCTION_NAMES: funcNames,//TODO USE it later when we start putting these in reports
}
return newQueryIssue(loFunctionsIssue, objectType, objectName, sqlStatement, details)
}
56 changes: 28 additions & 28 deletions yb-voyager/src/query/queryissue/parser_issue_detector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func TestDDLIssues(t *testing.T) {
},
stmt19: []QueryIssue{
NewLODatatypeIssue("TABLE", "test_lo_default", stmt19, "raster"),
NewLOFuntionsIssue("TABLE", "test_lo_default", stmt19),
NewLOFuntionsIssue("TABLE", "test_lo_default", stmt19, []string{"lo_import"}),
},
}
for _, stmt := range requiredDDLs {
Expand Down Expand Up @@ -415,31 +415,31 @@ BEGIN
END;
$$ LANGUAGE plpgsql;
`,
`CREATE TRIGGER t_raster BEFORE UPDATE OR DELETE ON image
`CREATE TRIGGER t_raster BEFORE UPDATE OR DELETE ON image
FOR EACH ROW EXECUTE FUNCTION lo_manage(raster);`,
}

expectedSQLsWithIssues := map[string][]QueryIssue{
sqls[0]: []QueryIssue{
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_unlink(loid);"),
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_unlink(loid);", []string{"lo_unlink"}),
},
sqls[1]: []QueryIssue{
NewLOFuntionsIssue("DML_QUERY", "", "INSERT INTO documents (title, content_oid) VALUES (doc_title, lo_import(file_path));"),
NewLOFuntionsIssue("DML_QUERY", "", "INSERT INTO documents (title, content_oid) VALUES (doc_title, lo_import(file_path));", []string{"lo_import"}),
},
sqls[2]: []QueryIssue{
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_export(loid, file_path);"),
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_export(loid, file_path);", []string{"lo_export"}),
},
sqls[3]: []QueryIssue{
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_close(fd);"),
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_close(fd);", []string{"lo_close"}),
},
sqls[4]: []QueryIssue{
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_put(fd, convert_to(new_data, 'UTF8'));"),
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_close(fd);"),
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_put(fd, convert_to(new_data, 'UTF8'));", []string{"lo_put"}),
NewLOFuntionsIssue("DML_QUERY", "", "SELECT lo_close(fd);", []string{"lo_close"}),
NewLODatatypeIssue("TABLE", "test_large_objects", "CREATE TABLE IF NOT EXISTS test_large_objects(id INT, raster lo DEFAULT lo_import(3242));", "raster"),
NewLOFuntionsIssue("TABLE", "test_large_objects", "CREATE TABLE IF NOT EXISTS test_large_objects(id INT, raster lo DEFAULT lo_import(3242));"),
NewLOFuntionsIssue("TABLE", "test_large_objects", "CREATE TABLE IF NOT EXISTS test_large_objects(id INT, raster lo DEFAULT lo_import(3242));", []string{"lo_import"}),
},
sqls[5]: []QueryIssue{
NewLOFuntionsIssue("TRIGGER", "t_raster ON image", sqls[5]),
NewLOFuntionsIssue("TRIGGER", "t_raster ON image", sqls[5], []string{"lo_manage"}),
},
}
expectedSQLsWithIssues[sqls[0]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[0]], "FUNCTION", "manage_large_object")
Expand All @@ -448,7 +448,6 @@ $$ LANGUAGE plpgsql;
expectedSQLsWithIssues[sqls[3]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[3]], "PROCEDURE", "read_large_object")
expectedSQLsWithIssues[sqls[4]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[4]], "FUNCTION", "write_to_large_object")


parserIssueDetector := NewParserIssueDetector()

for stmt, expectedIssues := range expectedSQLsWithIssues {
Expand All @@ -466,6 +465,7 @@ $$ LANGUAGE plpgsql;
}
}
}

// currently, both FuncCallDetector and XmlExprDetector can detect XMLFunctionsIssue
// statement below has both XML functions and XML expressions.
// but we want to only return one XMLFunctionsIssue from parserIssueDetector.getDMLIssues
Expand All @@ -491,13 +491,13 @@ func TestJsonUnsupportedFeatures(t *testing.T) {
`SELECT department, JSON_ARRAYAGG(name) AS employees_json
FROM employees
GROUP BY department;`,
`INSERT INTO movies (details)
`INSERT INTO movies (details)
VALUES (
JSON_OBJECT('title' VALUE 'Dune', 'director' VALUE 'Denis Villeneuve', 'year' VALUE 2021)
);`,
`SELECT json_objectagg(k VALUE v) AS json_result
FROM (VALUES ('a', 1), ('b', 2), ('c', 3)) AS t(k, v);`,
`SELECT JSON_OBJECT(
`SELECT JSON_OBJECT(
'movie' VALUE JSON_OBJECT('code' VALUE 'P123', 'title' VALUE 'Jaws'),
'director' VALUE 'Steven Spielberg'
) AS nested_json_object;`,
Expand All @@ -508,7 +508,7 @@ VALUES (
'price' VALUE 19.99,
'available' VALUE TRUE
) AS json_obj;`,
`SELECT id, JSON_QUERY(details, '$.author') AS author
`SELECT id, JSON_QUERY(details, '$.author') AS author
FROM books;`,
`SELECT jt.* FROM
my_films,
Expand All @@ -517,7 +517,7 @@ FROM books;`,
kind text PATH '$.kind',
title text PATH '$.films[*].title' WITH WRAPPER,
director text PATH '$.films[*].director' WITH WRAPPER)) AS jt;`,
`SELECT jt.* FROM
`SELECT jt.* FROM
my_films,
JSON_TABLE (js, $1 COLUMNS (
id FOR ORDINALITY,
Expand All @@ -534,50 +534,50 @@ FROM books;`,
JSON_VALUE(details, '$.title') AS title,
JSON_VALUE(details, '$.price')::NUMERIC AS price
FROM books;`,
`SELECT id, JSON_VALUE(details, '$.title') AS title
`SELECT id, JSON_VALUE(details, '$.title') AS title
FROM books
WHERE JSON_EXISTS(details, '$.price ? (@ > $price)' PASSING 30 AS price);`,
}
sqlsWithExpectedIssues := map[string][]QueryIssue{
sqls[0]: []QueryIssue{
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[0]),
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[0], []string{JSON_ARRAYAGG}),
},
sqls[1]: []QueryIssue{
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[1]),
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[1], []string{JSON_OBJECT}),
},
sqls[2]: []QueryIssue{
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[2]),
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[2], []string{JSON_OBJECTAGG}),
},
sqls[3]: []QueryIssue{
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[3]),
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[3], []string{JSON_OBJECT}),
},
sqls[4]: []QueryIssue{
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[4]),
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[4], []string{JSON_ARRAYAGG}),
},
sqls[5]: []QueryIssue{
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[5]),
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[5], []string{JSON_OBJECT}),
},
sqls[6]: []QueryIssue{
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[6]),
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[6], []string{JSON_QUERY}),
},
sqls[7]: []QueryIssue{
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[7]),
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[7], []string{JSON_TABLE}),
},
// sqls[8]: []QueryIssue{
// NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[8]),
//NOT REPORTED YET because of PARSER failing if JSON_TABLE has a parameterized values $1, $2 ...
// },
sqls[9]: []QueryIssue{
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[9]),
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[9], []string{JSON_EXISTS}),
},
sqls[10]: []QueryIssue{
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[10]),
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[10], []string{JSON_QUERY}),
},
sqls[11]: []QueryIssue{
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[11]),
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[11], []string{JSON_VALUE}),
},
sqls[12]: []QueryIssue{
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[12]),
NewJsonQueryFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[12], []string{JSON_VALUE, JSON_EXISTS}),
},
}
parserIssueDetector := NewParserIssueDetector()
Expand Down

0 comments on commit 1bb6de4

Please sign in to comment.