Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
priyanshi-yb committed Jan 20, 2025
1 parent fc1b227 commit 1eb486e
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 31 deletions.
2 changes: 2 additions & 0 deletions yb-voyager/cmd/assessMigrationCommand.go
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,8 @@ func fetchUnsupportedPGFeaturesFromSchemaReport(schemaAnalysisReport utils.Schem
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME, "", queryissue.FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, schemaAnalysisReport, false, ""))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.JSON_TYPE_PREDICATE_NAME, "", queryissue.JSON_TYPE_PREDICATE, schemaAnalysisReport, false, ""))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.SQL_BODY_IN_FUNCTION_NAME, "", queryissue.SQL_BODY_IN_FUNCTION, schemaAnalysisReport, false, ""))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.CTE_WITH_MATERIALIZED_CLAUSE_NAME, "", queryissue.CTE_WITH_MATERIALIZED_CLAUSE, schemaAnalysisReport, false, ""))

return lo.Filter(unsupportedFeatures, func(f UnsupportedFeature, _ int) bool {
return len(f.Objects) > 0
}), nil
Expand Down
1 change: 1 addition & 0 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ func (c *CommonTableExpressionDetector) Detect(msg protoreflect.Message) error {
return err
}
if cteNode.Ctematerialized != queryparser.CTE_MATERIALIZED_DEFAULT {
//MATERIALIZED / NOT MATERIALIZED clauses in CTE is not supported in YB
c.materializedClauseDetected = true
}
return nil
Expand Down
13 changes: 7 additions & 6 deletions yb-voyager/src/query/queryissue/issues_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,12 +567,13 @@ func NewForeignKeyReferencesPartitionedTableIssue(objectType string, objectName
}

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: "https://github.com/yugabyte/yugabyte-db/issues/25575",
DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
Type: SQL_BODY_IN_FUNCTION,
Name: SQL_BODY_IN_FUNCTION_NAME,
Impact: constants.IMPACT_LEVEL_1,
Description: "SQL Body for sql languages in function statement is not supported in YugabyteDB",
Suggestion: "No workaround available",
GH: "https://github.com/yugabyte/yugabyte-db/issues/25575",
DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
}

func NewSqlBodyInFunctionIssue(objectType string, objectName string, SqlStatement string) QueryIssue {
Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/src/query/queryissue/issues_dml.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ var cteWithMaterializedIssue = issue.Issue{
Type: CTE_WITH_MATERIALIZED_CLAUSE,
Name: CTE_WITH_MATERIALIZED_CLAUSE_NAME,
Impact: constants.IMPACT_LEVEL_2,
Description: "",
Description: "Modifying the materialization of CTE is not supported yet in YugabyteDB.",
Suggestion: "No workaround available right now",
GH: "https://github.com/yugabyte/yugabyte-db/issues/25575",
DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
Expand Down
30 changes: 29 additions & 1 deletion yb-voyager/src/query/queryissue/parser_issue_detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,27 @@ WHERE w2.key = 123;`,
)
INSERT INTO products_log
SELECT * FROM moved_rows;`,
`CREATE VIEW view1 AS
WITH data_cte AS NOT MATERIALIZED (
SELECT
generate_series(1, 5) AS id,
'Name ' || generate_series(1, 5) AS name
)
SELECT * FROM data_cte;`,
`CREATE VIEW view2 AS
WITH data_cte AS MATERIALIZED (
SELECT
generate_series(1, 5) AS id,
'Name ' || generate_series(1, 5) AS name
)
SELECT * FROM data_cte;`,
`CREATE VIEW view3 AS
WITH data_cte AS (
SELECT
generate_series(1, 5) AS id,
'Name ' || generate_series(1, 5) AS name
)
SELECT * FROM data_cte;`,
}

stmtsWithExpectedIssues := map[string][]QueryIssue{
Expand All @@ -1071,11 +1092,18 @@ SELECT * FROM moved_rows;`,
sqls[2]: []QueryIssue{
NewCTEWithMaterializedIssue(DML_QUERY_OBJECT_TYPE, "", sqls[2]),
},
sqls[3]: []QueryIssue{
NewCTEWithMaterializedIssue("VIEW", "view1", sqls[3]),
},
sqls[4]: []QueryIssue{
NewCTEWithMaterializedIssue("VIEW", "view2", sqls[4]),
},
sqls[5]: []QueryIssue{},
}

parserIssueDetector := NewParserIssueDetector()
for stmt, expectedIssues := range stmtsWithExpectedIssues {
issues, err := parserIssueDetector.GetDMLIssues(stmt, ybversion.LatestStable)
issues, err := parserIssueDetector.GetAllIssues(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)
Expand Down
27 changes: 4 additions & 23 deletions yb-voyager/src/query/queryparser/helpers_protomsg.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

pg_query "github.com/pganalyze/pg_query_go/v6"
log "github.com/sirupsen/logrus"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
)

Expand Down Expand Up @@ -401,36 +400,23 @@ func GetSchemaAndObjectName(nameList protoreflect.List) (string, string) {
}

func ProtoAsSelectStmt(msg protoreflect.Message) (*pg_query.SelectStmt, error) {
protoMsg, ok := msg.Interface().(proto.Message)
if !ok {
return nil, fmt.Errorf("failed to cast msg to proto.Message")
}
selectStmtNode, ok := protoMsg.(*pg_query.SelectStmt)
selectStmtNode, ok := msg.Interface().(*pg_query.SelectStmt)
if !ok {
return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_SELECTSTMT_NODE)
}
return selectStmtNode, nil
}

func ProtoAsCTENode(msg protoreflect.Message) (*pg_query.CommonTableExpr, error) {
protoMsg, ok := msg.Interface().(proto.Message)
if !ok {
return nil, fmt.Errorf("failed to cast msg to proto.Message")
}
cteNode, ok := protoMsg.(*pg_query.CommonTableExpr)
cteNode, ok := msg.Interface().(*pg_query.CommonTableExpr)
if !ok {
return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_CTE_NODE)
}
return cteNode, nil
}

func ProtoAsIndexStmt(msg protoreflect.Message) (*pg_query.IndexStmt, error) {
protoMsg, ok := msg.Interface().(proto.Message)
if !ok {
return nil, fmt.Errorf("failed to cast msg to proto.Message")
}

indexStmtNode, ok := protoMsg.(*pg_query.IndexStmt)
indexStmtNode, ok := msg.Interface().(*pg_query.IndexStmt)
if !ok {
return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_INDEX_STMT_NODE)
}
Expand All @@ -439,12 +425,7 @@ func ProtoAsIndexStmt(msg protoreflect.Message) (*pg_query.IndexStmt, error) {
}

func ProtoAsTableConstraint(msg protoreflect.Message) (*pg_query.Constraint, error) {
protoMsg, ok := msg.Interface().(proto.Message)
if !ok {
return nil, fmt.Errorf("failed to cast msg to proto.Message")
}

consNode, ok := protoMsg.(*pg_query.Constraint)
consNode, ok := msg.Interface().(*pg_query.Constraint)
if !ok {
return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_CONSTRAINT_NODE)
}
Expand Down

0 comments on commit 1eb486e

Please sign in to comment.