Skip to content

Commit

Permalink
minor review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
priyanshi-yb committed Jan 6, 2025
1 parent 1d666b1 commit 29491dd
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 20 deletions.
16 changes: 8 additions & 8 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,22 +403,22 @@ func NewJsonPredicateExprDetector(query string) *JsonPredicateExprDetector {
}

func (j *JsonPredicateExprDetector) Detect(msg protoreflect.Message) error {
if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_JSON_PREDICATE_NODE {
if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_JSON_IS_PREDICATE_NODE {
/*
SELECT js IS JSON "json?" FROM (VALUES ('123')) foo(js);
stmts:{stmt:{select_stmt:{target_list:{res_target:{val:{column_ref:{fields:{string:{sval:"js"}} location:337}} location:337}}
target_list:{res_target:{name:"json?" val:{json_is_predicate:{expr:{column_ref:{fields:{string:{sval:"js"}} location:341}}
format:{format_type:JS_FORMAT_DEFAULT encoding:JS_ENC_DEFAULT location:-1} item_type:JS_TYPE_ANY location:341}} location:341}} ...
SELECT js IS JSON "json?" FROM (VALUES ('123')) foo(js);
stmts:{stmt:{select_stmt:{target_list:{res_target:{val:{column_ref:{fields:{string:{sval:"js"}} location:337}} location:337}}
target_list:{res_target:{name:"json?" val:{json_is_predicate:{expr:{column_ref:{fields:{string:{sval:"js"}} location:341}}
format:{format_type:JS_FORMAT_DEFAULT encoding:JS_ENC_DEFAULT location:-1} item_type:JS_TYPE_ANY location:341}} location:341}} ...
*/
j.detected = true
}
return nil
}

func (d *JsonPredicateExprDetector) GetIssues() []QueryIssue {
func (j *JsonPredicateExprDetector) GetIssues() []QueryIssue {
var issues []QueryIssue
if d.detected {
issues = append(issues, NewJsonPredicateIssue(DML_QUERY_OBJECT_TYPE, "", d.query))
if j.detected {
issues = append(issues, NewJsonPredicateIssue(DML_QUERY_OBJECT_TYPE, "", j.query))
}
return issues
}
20 changes: 9 additions & 11 deletions yb-voyager/src/query/queryissue/parser_issue_detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,11 @@ JSON_TABLE(data, '$.skills[*]'
)
) AS jt;`,
`SELECT JSON_ARRAY($1, 12, TRUE, $2) AS json_array;`,
`CREATE TABLE sales.json_data (
id int PRIMARY KEY,
array_column TEXT CHECK (array_column IS JSON ARRAY),
unique_keys_column TEXT CHECK (unique_keys_column IS JSON WITH UNIQUE KEYS)
);`,
}
sqlsWithExpectedIssues := map[string][]QueryIssue{
sqls[0]: []QueryIssue{
Expand Down Expand Up @@ -631,6 +636,9 @@ JSON_TABLE(data, '$.skills[*]'
sqls[17]: []QueryIssue{
NewJsonConstructorFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[17], []string{JSON_ARRAY}),
},
sqls[18]: []QueryIssue{
NewJsonPredicateIssue("TABLE", "sales.json_data", sqls[18]),
},
}
parserIssueDetector := NewParserIssueDetector()
for stmt, expectedIssues := range sqlsWithExpectedIssues {
Expand All @@ -657,10 +665,6 @@ func TestAggregateFunctions(t *testing.T) {
FROM multiranges;`,
`SELECT range_agg(multi_event_range) AS union_of_multiranges
FROM multiranges;`,
`SELECT range_intersect_agg(event_range) AS intersection_of_ranges
FROM events;`,
`SELECT range_agg(event_range) AS union_of_ranges
FROM events;`,
`CREATE OR REPLACE FUNCTION aggregate_ranges()
RETURNS INT4MULTIRANGE AS $$
DECLARE
Expand All @@ -687,17 +691,11 @@ $$ LANGUAGE plpgsql;`,
NewAggregationFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[2], []string{"range_agg"}),
},
sqls[3]: []QueryIssue{
NewAggregationFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[3], []string{"range_intersect_agg"}),
},
sqls[4]: []QueryIssue{
NewAggregationFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[4], []string{"range_agg"}),
},
sqls[5]: []QueryIssue{
NewAggregationFunctionIssue(DML_QUERY_OBJECT_TYPE, "", "SELECT range_agg(range_value) FROM ranges;", []string{"range_agg"}),
NewAggregationFunctionIssue(DML_QUERY_OBJECT_TYPE, "", sqls[0], []string{"any_value"}),
},
}
aggregateSqls[sqls[5]] = modifiedIssuesforPLPGSQL(aggregateSqls[sqls[5]], "FUNCTION", "aggregate_ranges")
aggregateSqls[sqls[3]] = modifiedIssuesforPLPGSQL(aggregateSqls[sqls[3]], "FUNCTION", "aggregate_ranges")

parserIssueDetector := NewParserIssueDetector()
for stmt, expectedIssues := range aggregateSqls {
Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/src/query/queryparser/traversal_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const (
PG_QUERY_JSON_FUNC_EXPR_NODE = "pg_query.JsonFuncExpr"
PG_QUERY_JSON_OBJECT_CONSTRUCTOR_NODE = "pg_query.JsonObjectConstructor"
PG_QUERY_JSON_TABLE_NODE = "pg_query.JsonTable"
PG_QUERY_JSON_PREDICATE_NODE = "pg_query.JsonIsPredicate"
PG_QUERY_JSON_IS_PREDICATE_NODE = "pg_query.JsonIsPredicate"
PG_QUERY_VIEWSTMT_NODE = "pg_query.ViewStmt"
PG_QUERY_COPYSTSMT_NODE = "pg_query.CopyStmt"
)
Expand Down

0 comments on commit 29491dd

Please sign in to comment.