Skip to content

Commit

Permalink
reporting SQL body and added unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
priyanshi-yb committed Jan 8, 2025
1 parent 15b4e98 commit a9ab725
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 2 deletions.
5 changes: 4 additions & 1 deletion yb-voyager/src/query/queryissue/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,11 @@ const (
FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE = "FOREIGN_KEY_REFERENCED_PARTITIONED_TABLE"
FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME = "Foreign key constraint references partitioned table"

CTE_WITH_MATERIALIZED_CLAUSE = "CTE_WITH_MATERIALIZED_CLAUSE"
CTE_WITH_MATERIALIZED_CLAUSE = "CTE_WITH_MATERIALIZED_CLAUSE"
CTE_WITH_MATERIALIZED_CLAUSE_NAME = "Modifying Materialization of Common table expressions not supported"

SQL_BODY_IN_FUNCTION = "SQL_BODY_IN_FUNCTION"
SQL_BODY_IN_FUNCTION_NAME = "SQL Body in function"
)

// Object types
Expand Down
5 changes: 4 additions & 1 deletion yb-voyager/src/query/queryissue/detectors_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,8 @@ func (v *FunctionIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]Query
var issues []QueryIssue

if function.HasSqlBody {
// issues = append(issues)
//https://www.postgresql.org/docs/15/sql-createfunction.html#:~:text=a%20new%20session.-,sql_body,-The%20body%20of
issues = append(issues, NewSqlBodyInFunctionIssue(function.GetObjectType(), function.GetObjectName(), ""))
}

return issues, nil
Expand Down Expand Up @@ -664,6 +665,8 @@ func (p *ParserIssueDetector) GetDDLDetector(obj queryparser.DDLObject) (DDLIssu
return &ViewIssueDetector{}, nil
case *queryparser.MView:
return &MViewIssueDetector{}, nil
case *queryparser.Function:
return &FunctionIssueDetector{}, nil
default:
return &NoOpIssueDetector{}, nil
}
Expand Down
13 changes: 13 additions & 0 deletions yb-voyager/src/query/queryissue/issues_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,16 @@ func NewForeignKeyReferencesPartitionedTableIssue(objectType string, objectName
}
return newQueryIssue(foreignKeyReferencesPartitionedTableIssue, objectType, objectName, SqlStatement, details)
}

var sqlBodyInFunctionIssue = issue.Issue{
Type: SQL_BODY_IN_FUNCTION,
Name: SQL_BODY_IN_FUNCTION_NAME,
Impact: constants.IMPACT_LEVEL_1,
Suggestion: "No workaround available",
GH: "",
DocsLink: "",
}

func NewSqlBodyInFunctionIssue(objectType string, objectName string, SqlStatement string) QueryIssue {
return newQueryIssue(sqlBodyInFunctionIssue, objectType, objectName, SqlStatement, map[string]interface{}{})
}
28 changes: 28 additions & 0 deletions yb-voyager/src/query/queryissue/issues_ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,31 @@ func testForeignKeyReferencesPartitionedTableIssue(t *testing.T) {
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, `cannot reference partitioned table "abc1"`, foreignKeyReferencesPartitionedTableIssue)
}

func testSQLBodyInFunctionIssue(t *testing.T) {
sqls := map[string]string{
`CREATE OR REPLACE FUNCTION asterisks(n int)
RETURNS text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
RETURN repeat('*', n);`: `syntax error at or near "RETURN"`
`CREATE OR REPLACE FUNCTION asterisks(n int)
RETURNS SETOF text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
BEGIN ATOMIC
SELECT repeat('*', g) FROM generate_series (1, n) g;
END;`: `syntax error at or near "BEGIN"`
}
for sql, errMsg := range sqls {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, sql)

assertErrorCorrectlyThrownForIssueForYBVersion(t, err, errMsg, sqlBodyInFunctionIssue)
}
}

func TestDDLIssuesInYBVersion(t *testing.T) {
var err error
ybVersion := os.Getenv("YB_VERSION")
Expand Down Expand Up @@ -348,4 +373,7 @@ func TestDDLIssuesInYBVersion(t *testing.T) {

success = t.Run(fmt.Sprintf("%s-%s", "foreign key referenced partitioned table", ybVersion), testForeignKeyReferencesPartitionedTableIssue)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "sql body in function", ybVersion), testSQLBodyInFunctionIssue)
assert.True(t, success)
}
31 changes: 31 additions & 0 deletions yb-voyager/src/query/queryissue/issues_dml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,34 @@ FROM events;`,
}
}

func testCTEWithMaterializedIssue(t *testing.T) {
sqls := map[string]string{`WITH w AS NOT MATERIALIZED (
SELECT * FROM big_table
)
SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
WHERE w2.key = 123;`: `syntax error at or near "NOT"`
`WITH moved_rows AS MATERIALIZED (
DELETE FROM products
WHERE
"date" >= '2010-10-01' AND
"date" < '2010-11-01'
RETURNING *
)
INSERT INTO products_log
SELECT * FROM moved_rows;`: `syntax error at or near "MATERIALIZED"`
}
for sql, errMsg := range sqls {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, sql)

assertErrorCorrectlyThrownForIssueForYBVersion(t, err, errMsg, cteWithMaterializedIssue)
}
}

func TestDMLIssuesInYBVersion(t *testing.T) {
var err error
ybVersion := os.Getenv("YB_VERSION")
Expand Down Expand Up @@ -283,4 +311,7 @@ func TestDMLIssuesInYBVersion(t *testing.T) {
success = t.Run(fmt.Sprintf("%s-%s", "json type predicate", ybVersion), testJsonPredicateIssue)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "cte with materialized cluase", ybVersion), testCTEWithMaterializedIssue)
assert.True(t, success)

}
1 change: 1 addition & 0 deletions yb-voyager/src/query/queryissue/parser_issue_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ func (p *ParserIssueDetector) genericIssues(query string) ([]QueryIssue, error)
NewJsonQueryFunctionDetector(query),
NewJsonbSubscriptingDetector(query, p.jsonbColumns, p.getJsonbReturnTypeFunctions()),
NewJsonPredicateExprDetector(query),
NewCommonTableExpressionDetector(query),
}

processor := func(msg protoreflect.Message) error {
Expand Down
92 changes: 92 additions & 0 deletions yb-voyager/src/query/queryissue/parser_issue_detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1009,3 +1009,95 @@ REFERENCES schema1.abc (id);
}
}
}

func TestCTEIssues(t *testing.T) {
sqls := []string{
`WITH w AS (
SELECT key, very_expensive_function(val) as f FROM some_table
)
SELECT * FROM w AS w1 JOIN w AS w2 ON w1.f = w2.f;`,
`WITH w AS NOT MATERIALIZED (
SELECT * FROM big_table
)
SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
WHERE w2.key = 123;`,
`WITH moved_rows AS MATERIALIZED (
DELETE FROM products
WHERE
"date" >= '2010-10-01' AND
"date" < '2010-11-01'
RETURNING *
)
INSERT INTO products_log
SELECT * FROM moved_rows;`,
}

stmtsWithExpectedIssues := map[string][]QueryIssue{
sqls[0]: []QueryIssue{},
sqls[1]: []QueryIssue{
NewCTEWithMaterializedIssue(DML_QUERY_OBJECT_TYPE, "", sqls[1]),
},
sqls[2]: []QueryIssue{
NewCTEWithMaterializedIssue(DML_QUERY_OBJECT_TYPE, "", sqls[2]),
},
}

parserIssueDetector := NewParserIssueDetector()
for stmt, expectedIssues := range stmtsWithExpectedIssues {
issues, err := parserIssueDetector.GetDMLIssues(stmt, ybversion.LatestStable)
assert.NoError(t, err, "Error detecting issues for statement: %s", stmt)

assert.Equal(t, len(expectedIssues), len(issues), "Mismatch in issue count for statement: %s", stmt)
for _, expectedIssue := range expectedIssues {
found := slices.ContainsFunc(issues, func(queryIssue QueryIssue) bool {
return cmp.Equal(expectedIssue, queryIssue)
})
assert.True(t, found, "Expected issue not found: %v in statement: %s", expectedIssue, stmt)
}
}

}

func TestSQLBodyIssues(t *testing.T) {
sqls := []string{
`CREATE OR REPLACE FUNCTION asterisks(n int)
RETURNS text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
RETURN repeat('*', n);`,
`CREATE OR REPLACE FUNCTION asterisks(n int)
RETURNS SETOF text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
BEGIN ATOMIC
SELECT repeat('*', g) FROM generate_series (1, n) g;
END;`,
`CREATE OR REPLACE FUNCTION asterisks(n int)
RETURNS SETOF text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE AS
$func$
SELECT repeat('*', g) FROM generate_series (1, n) g;
$func$;`,
}

stmtsWithExpectedIssues := map[string][]QueryIssue{
sqls[0]: []QueryIssue{
NewSqlBodyInFunctionIssue("FUNCTION", "asterisks", sqls[0]),
},
sqls[1]: []QueryIssue{
NewSqlBodyInFunctionIssue("FUNCTION", "asterisks", sqls[1]),
},
sqls[2]: []QueryIssue{},
}
parserIssueDetector := NewParserIssueDetector()
for stmt, expectedIssues := range stmtsWithExpectedIssues {
issues, err := parserIssueDetector.GetDDLIssues(stmt, ybversion.LatestStable)
assert.NoError(t, err, "Error detecting issues for statement: %s", stmt)

assert.Equal(t, len(expectedIssues), len(issues), "Mismatch in issue count for statement: %s", stmt)
for _, expectedIssue := range expectedIssues {
found := slices.ContainsFunc(issues, func(queryIssue QueryIssue) bool {
return cmp.Equal(expectedIssue, queryIssue)
})
assert.True(t, found, "Expected issue not found: %v in statement: %s", expectedIssue, stmt)
}
}
}

0 comments on commit a9ab725

Please sign in to comment.