From fd998d578c2a439c3e4c7ef94b576c3462713874 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 20 Dec 2024 10:41:39 +0530 Subject: [PATCH 01/18] basic issue detection --- yb-voyager/go.mod | 3 +- yb-voyager/go.sum | 1 - yb-voyager/src/query/queryissue/constants.go | 1 + yb-voyager/src/query/queryissue/detectors.go | 58 +++++++++++++++++++ yb-voyager/src/query/queryissue/issues_dml.go | 13 +++++ .../query/queryissue/parser_issue_detector.go | 1 + .../queryissue/parser_issue_detector_test.go | 16 ++++- .../src/query/queryparser/traversal_proto.go | 25 +++++++- 8 files changed, 112 insertions(+), 6 deletions(-) diff --git a/yb-voyager/go.mod b/yb-voyager/go.mod index c833375e2..57aa87840 100644 --- a/yb-voyager/go.mod +++ b/yb-voyager/go.mod @@ -11,6 +11,7 @@ require ( github.com/aws/aws-sdk-go-v2/config v1.18.15 github.com/aws/aws-sdk-go-v2/service/s3 v1.30.5 github.com/davecgh/go-spew v1.1.1 + github.com/deckarep/golang-set/v2 v2.7.0 github.com/docker/go-connections v0.5.0 github.com/dustin/go-humanize v1.0.1 github.com/fatih/color v1.13.0 @@ -44,6 +45,7 @@ require ( golang.org/x/term v0.24.0 google.golang.org/api v0.169.0 gopkg.in/natefinch/lumberjack.v2 v2.2.1 + gotest.tools/v3 v3.5.1 ) require ( @@ -55,7 +57,6 @@ require ( github.com/containerd/log v0.1.0 // indirect github.com/containerd/platforms v0.2.1 // indirect github.com/cpuguy83/dockercfg v0.3.2 // indirect - github.com/deckarep/golang-set/v2 v2.7.0 // indirect github.com/distribution/reference v0.6.0 // indirect github.com/docker/docker v27.1.1+incompatible // indirect github.com/docker/go-units v0.5.0 // indirect diff --git a/yb-voyager/go.sum b/yb-voyager/go.sum index 44ee2af8d..f94df5d12 100644 --- a/yb-voyager/go.sum +++ b/yb-voyager/go.sum @@ -2986,7 +2986,6 @@ gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= diff --git a/yb-voyager/src/query/queryissue/constants.go b/yb-voyager/src/query/queryissue/constants.go index e07367a38..0225715d0 100644 --- a/yb-voyager/src/query/queryissue/constants.go +++ b/yb-voyager/src/query/queryissue/constants.go @@ -59,6 +59,7 @@ const ( ADVISORY_LOCKS_NAME = "Advisory Locks" SYSTEM_COLUMNS_NAME = "System Columns" XML_FUNCTIONS_NAME = "XML Functions" + LIMIT_WITH_TIES = "LIMIT_WITH_TIES" ) // Object types diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index 228496a69..0c230ec06 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -16,11 +16,16 @@ limitations under the License. package queryissue import ( + "fmt" + mapset "github.com/deckarep/golang-set/v2" + pg_query "github.com/pganalyze/pg_query_go/v5" log "github.com/sirupsen/logrus" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "github.com/yugabyte/yb-voyager/yb-voyager/src/query/queryparser" + "github.com/yugabyte/yb-voyager/yb-voyager/src/utils" ) // To Add a new unsupported query construct implement this interface for all possible nodes for that construct @@ -53,6 +58,17 @@ func (d *FuncCallDetector) Detect(msg protoreflect.Message) error { return nil } + protoMsg := msg.Interface().(proto.Message) + funcCallNode, ok := protoMsg.(*pg_query.FuncCall) + if !ok { + return fmt.Errorf("failed to cast %s to %s", queryparser.PG_QUERY_FUNCCALL_NODE, queryparser.PG_QUERY_FUNCCALL_NODE) + } + funcNames := funcCallNode.Funcname + for _, funcName := range funcNames { + log.Infof("fetched function name from %s node: %q", queryparser.PG_QUERY_FUNCCALL_NODE, funcName.String()) + utils.PrintAndLog("funcName = %s", funcName.Node.(*pg_query.Node_String_).String_.Sval) + } + _, funcName := queryparser.GetFuncNameFromFuncCall(msg) log.Debugf("fetched function name from %s node: %q", queryparser.PG_QUERY_FUNCCALL_NODE, funcName) @@ -187,3 +203,45 @@ func (d *RangeTableFuncDetector) GetIssues() []QueryIssue { } return issues } + +type SelectStmtDetector struct { + query string + limitOptionWithTiesDetected bool +} + +func NewSelectStmtDetector(query string) *SelectStmtDetector { + return &SelectStmtDetector{ + query: query, + } +} + +func (d *SelectStmtDetector) getSelectStmtFromProto(msg protoreflect.Message) (*pg_query.SelectStmt, error) { + protoMsg := msg.Interface().(proto.Message) + selectStmtNode, ok := protoMsg.(*pg_query.SelectStmt) + if !ok { + return nil, fmt.Errorf("failed to cast %s to %s", queryparser.PG_QUERY_SELECTSTMT_NODE, queryparser.PG_QUERY_SELECTSTMT_NODE) + } + return selectStmtNode, nil +} + +// Detect checks if a SelectStmt node uses a LIMIT clause with TIES +func (d *SelectStmtDetector) Detect(msg protoreflect.Message) error { + if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_SELECTSTMT_NODE { + selectStmtNode, err := d.getSelectStmtFromProto(msg) + if err != nil { + return err + } + if selectStmtNode.LimitOption == pg_query.LimitOption_LIMIT_OPTION_WITH_TIES { + d.limitOptionWithTiesDetected = true + } + } + return nil +} + +func (d *SelectStmtDetector) GetIssues() []QueryIssue { + var issues []QueryIssue + if d.limitOptionWithTiesDetected { + issues = append(issues, NewLimitWithTiesIssue(DML_QUERY_OBJECT_TYPE, "", d.query)) + } + return issues +} diff --git a/yb-voyager/src/query/queryissue/issues_dml.go b/yb-voyager/src/query/queryissue/issues_dml.go index 3528ead45..4aef77390 100644 --- a/yb-voyager/src/query/queryissue/issues_dml.go +++ b/yb-voyager/src/query/queryissue/issues_dml.go @@ -69,3 +69,16 @@ var loFunctionsIssue = issue.Issue{ func NewLOFuntionsIssue(objectType string, objectName string, sqlStatement string) QueryIssue { return newQueryIssue(loFunctionsIssue, objectType, objectName, sqlStatement, map[string]interface{}{}) } + +var limitWithTiesIssue = issue.Issue{ + Type: LIMIT_WITH_TIES, + TypeName: "LIMIT .. WITH TIES", + TypeDescription: "LIMIT WITH TIES is not supported in YugabyteDB", + Suggestion: "LIMIT WITH TIES is not yet supported in YugabyteDB, no workaround available right now", + GH: "", + DocsLink: "", //TODO +} + +func NewLimitWithTiesIssue(objectType string, objectName string, sqlStatement string) QueryIssue { + return newQueryIssue(limitWithTiesIssue, objectType, objectName, sqlStatement, map[string]interface{}{}) +} diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector.go b/yb-voyager/src/query/queryissue/parser_issue_detector.go index ffa1d9860..c9a28c5d4 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector.go @@ -375,6 +375,7 @@ func (p *ParserIssueDetector) genericIssues(query string) ([]QueryIssue, error) NewColumnRefDetector(query), NewXmlExprDetector(query), NewRangeTableFuncDetector(query), + NewSelectStmtDetector(query), } processor := func(msg protoreflect.Message) error { diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go index 9ba4dac4d..72a3e3695 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -415,7 +415,7 @@ 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);`, } @@ -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 { @@ -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 @@ -485,3 +485,15 @@ func TestSingleXMLIssueIsDetected(t *testing.T) { fatalIfError(t, err) assert.Equal(t, 1, len(issues)) } + +func TestWithTies(t *testing.T) { + stmt := ` + SELECT * FROM employees + ORDER BY salary DESC + FETCH FIRST 2 ROWS WITH TIES;` + parserIssueDetector := NewParserIssueDetector() + issues, err := parserIssueDetector.getDMLIssues(stmt) + fatalIfError(t, err) + assert.Equal(t, 1, len(issues)) + assert.True(t, cmp.Equal(issues[0], NewLimitWithTiesIssue("DML_QUERY", "", stmt))) +} diff --git a/yb-voyager/src/query/queryparser/traversal_proto.go b/yb-voyager/src/query/queryparser/traversal_proto.go index 3ed84f4ab..c148a48a2 100644 --- a/yb-voyager/src/query/queryparser/traversal_proto.go +++ b/yb-voyager/src/query/queryparser/traversal_proto.go @@ -20,6 +20,7 @@ import ( "slices" log "github.com/sirupsen/logrus" + "github.com/yugabyte/yb-voyager/yb-voyager/src/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -40,6 +41,7 @@ const ( PG_QUERY_INSERTSTMT_NODE = "pg_query.InsertStmt" PG_QUERY_UPDATESTMT_NODE = "pg_query.UpdateStmt" PG_QUERY_DELETESTMT_NODE = "pg_query.DeleteStmt" + PG_QUERY_SELECTSTMT_NODE = "pg_query.SelectStmt" ) // function type for processing nodes during traversal @@ -113,7 +115,7 @@ func TraverseParseTree(msg protoreflect.Message, visited map[protoreflect.Messag visited[msg] = true nodeType := msg.Descriptor().FullName() - log.Debugf("Traversing NodeType: %s\n", nodeType) + utils.PrintAndLog("Traversing NodeType: %s\n", nodeType) // applying the processor to the current node if err := processor(msg); err != nil { log.Debugf("error processing node %s: %v", nodeType, err) @@ -167,7 +169,26 @@ func TraverseNodeFields(msg protoreflect.Message, visited map[protoreflect.Messa continue // Skip unset fields in the oneof group } } - log.Debugf("Field: %s, Type: %s\n", fieldDesc.Name(), fieldDesc.Kind()) + utils.PrintAndLog("Field: %s, Type: %s\n", fieldDesc.Name(), fieldDesc.Kind()) + // if fieldDesc.Name() == "limit_option" { + // protoMsg := msg.Interface().(proto.Message) + // selectStmt, ok := protoMsg.(*pg_query.SelectStmt) + // if !ok { + // return fmt.Errorf("error casting message to SelectStmt: %v", selectStmt) + // } + + // // bytes, err := proto.Marshal(msg.Interface().(proto.Message)) + // // if err != nil { + // // return fmt.Errorf("error marshalling message: %w", err) + // // } + // // var selectStmt pg_query.SelectStmt + // // err = proto.Unmarshal(bytes, &selectStmt) + // // if err != nil { + // // return fmt.Errorf("error unmarshalling message: %w", err) + // // } + + // utils.PrintAndLog("Field: %s, Type: %s\n", fieldDesc.Name(), fieldDesc.Kind()) + // } value := msg.Get(fieldDesc) switch { case fieldDesc.IsList() && fieldDesc.Kind() == protoreflect.MessageKind: From e4e0408719f42337d6933f387abf9306f772384f Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Mon, 23 Dec 2024 11:07:20 +0530 Subject: [PATCH 02/18] minor cleanup --- yb-voyager/src/query/queryissue/detectors.go | 14 +--------- .../queryissue/parser_issue_detector_test.go | 26 +++++++++++++++---- .../src/query/queryparser/traversal_proto.go | 24 ++--------------- 3 files changed, 24 insertions(+), 40 deletions(-) diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index 0c230ec06..fc101a659 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -25,7 +25,6 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" "github.com/yugabyte/yb-voyager/yb-voyager/src/query/queryparser" - "github.com/yugabyte/yb-voyager/yb-voyager/src/utils" ) // To Add a new unsupported query construct implement this interface for all possible nodes for that construct @@ -58,17 +57,6 @@ func (d *FuncCallDetector) Detect(msg protoreflect.Message) error { return nil } - protoMsg := msg.Interface().(proto.Message) - funcCallNode, ok := protoMsg.(*pg_query.FuncCall) - if !ok { - return fmt.Errorf("failed to cast %s to %s", queryparser.PG_QUERY_FUNCCALL_NODE, queryparser.PG_QUERY_FUNCCALL_NODE) - } - funcNames := funcCallNode.Funcname - for _, funcName := range funcNames { - log.Infof("fetched function name from %s node: %q", queryparser.PG_QUERY_FUNCCALL_NODE, funcName.String()) - utils.PrintAndLog("funcName = %s", funcName.Node.(*pg_query.Node_String_).String_.Sval) - } - _, funcName := queryparser.GetFuncNameFromFuncCall(msg) log.Debugf("fetched function name from %s node: %q", queryparser.PG_QUERY_FUNCCALL_NODE, funcName) @@ -224,13 +212,13 @@ func (d *SelectStmtDetector) getSelectStmtFromProto(msg protoreflect.Message) (* return selectStmtNode, nil } -// Detect checks if a SelectStmt node uses a LIMIT clause with TIES func (d *SelectStmtDetector) Detect(msg protoreflect.Message) error { if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_SELECTSTMT_NODE { selectStmtNode, err := d.getSelectStmtFromProto(msg) if err != nil { return err } + // checks if a SelectStmt node uses a LIMIT clause with TIES if selectStmtNode.LimitOption == pg_query.LimitOption_LIMIT_OPTION_WITH_TIES { d.limitOptionWithTiesDetected = true } diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go index 72a3e3695..5a17c281c 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -487,13 +487,29 @@ func TestSingleXMLIssueIsDetected(t *testing.T) { } func TestWithTies(t *testing.T) { - stmt := ` + + stmt1 := ` SELECT * FROM employees ORDER BY salary DESC FETCH FIRST 2 ROWS WITH TIES;` + + expectedIssues := map[string][]QueryIssue{ + stmt1: []QueryIssue{NewLimitWithTiesIssue("DML_QUERY", "", stmt1)}, + } + parserIssueDetector := NewParserIssueDetector() - issues, err := parserIssueDetector.getDMLIssues(stmt) - fatalIfError(t, err) - assert.Equal(t, 1, len(issues)) - assert.True(t, cmp.Equal(issues[0], NewLimitWithTiesIssue("DML_QUERY", "", stmt))) + + for stmt, expectedIssues := range expectedIssues { + 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) + 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) + } + } } diff --git a/yb-voyager/src/query/queryparser/traversal_proto.go b/yb-voyager/src/query/queryparser/traversal_proto.go index c148a48a2..6babb72a8 100644 --- a/yb-voyager/src/query/queryparser/traversal_proto.go +++ b/yb-voyager/src/query/queryparser/traversal_proto.go @@ -20,7 +20,6 @@ import ( "slices" log "github.com/sirupsen/logrus" - "github.com/yugabyte/yb-voyager/yb-voyager/src/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -115,7 +114,7 @@ func TraverseParseTree(msg protoreflect.Message, visited map[protoreflect.Messag visited[msg] = true nodeType := msg.Descriptor().FullName() - utils.PrintAndLog("Traversing NodeType: %s\n", nodeType) + log.Debugf("Traversing NodeType: %s\n", nodeType) // applying the processor to the current node if err := processor(msg); err != nil { log.Debugf("error processing node %s: %v", nodeType, err) @@ -169,26 +168,7 @@ func TraverseNodeFields(msg protoreflect.Message, visited map[protoreflect.Messa continue // Skip unset fields in the oneof group } } - utils.PrintAndLog("Field: %s, Type: %s\n", fieldDesc.Name(), fieldDesc.Kind()) - // if fieldDesc.Name() == "limit_option" { - // protoMsg := msg.Interface().(proto.Message) - // selectStmt, ok := protoMsg.(*pg_query.SelectStmt) - // if !ok { - // return fmt.Errorf("error casting message to SelectStmt: %v", selectStmt) - // } - - // // bytes, err := proto.Marshal(msg.Interface().(proto.Message)) - // // if err != nil { - // // return fmt.Errorf("error marshalling message: %w", err) - // // } - // // var selectStmt pg_query.SelectStmt - // // err = proto.Unmarshal(bytes, &selectStmt) - // // if err != nil { - // // return fmt.Errorf("error unmarshalling message: %w", err) - // // } - - // utils.PrintAndLog("Field: %s, Type: %s\n", fieldDesc.Name(), fieldDesc.Kind()) - // } + log.Debugf("Field: %s, Type: %s\n", fieldDesc.Name(), fieldDesc.Kind()) value := msg.Get(fieldDesc) switch { case fieldDesc.IsList() && fieldDesc.Kind() == protoreflect.MessageKind: From d3f4cdf42c1d10dac48f67e1ad914cf9af5274ff Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Mon, 23 Dec 2024 11:28:31 +0530 Subject: [PATCH 03/18] tests --- .../queryissue/parser_issue_detector_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go index 5a17c281c..58b615304 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -493,8 +493,26 @@ func TestWithTies(t *testing.T) { ORDER BY salary DESC FETCH FIRST 2 ROWS WITH TIES;` + // subquery + stmt2 := `SELECT * + FROM ( + SELECT * FROM employees + ORDER BY salary DESC + FETCH FIRST 2 ROWS WITH TIES + ) AS top_employees;` + + stmt3 := `CREATE VIEW top_employees_view AS + SELECT * + FROM ( + SELECT * FROM employees + ORDER BY salary DESC + FETCH FIRST 2 ROWS WITH TIES + ) AS top_employees;` + expectedIssues := map[string][]QueryIssue{ stmt1: []QueryIssue{NewLimitWithTiesIssue("DML_QUERY", "", stmt1)}, + stmt2: []QueryIssue{NewLimitWithTiesIssue("DML_QUERY", "", stmt2)}, + stmt3: []QueryIssue{NewLimitWithTiesIssue("VIEW", "top_employees_view", stmt3)}, } parserIssueDetector := NewParserIssueDetector() From ec88f04c48a5b8db2f4659db473bad6a6403cf00 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Mon, 23 Dec 2024 14:58:15 +0530 Subject: [PATCH 04/18] v6 --- yb-voyager/src/query/queryissue/detectors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index fc101a659..0e87f1db9 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -19,7 +19,7 @@ import ( "fmt" mapset "github.com/deckarep/golang-set/v2" - pg_query "github.com/pganalyze/pg_query_go/v5" + 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" From 920622d49c3e9963c3188a50646db52d4620e403 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Mon, 23 Dec 2024 16:19:52 +0530 Subject: [PATCH 05/18] rename --- yb-voyager/src/query/queryissue/constants.go | 2 +- yb-voyager/src/query/queryissue/detectors.go | 5 +++-- yb-voyager/src/query/queryissue/issues_dml.go | 14 +++++++------- .../query/queryissue/parser_issue_detector_test.go | 6 +++--- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/yb-voyager/src/query/queryissue/constants.go b/yb-voyager/src/query/queryissue/constants.go index 4792fe646..107ad4e0c 100644 --- a/yb-voyager/src/query/queryissue/constants.go +++ b/yb-voyager/src/query/queryissue/constants.go @@ -59,7 +59,7 @@ const ( ADVISORY_LOCKS_NAME = "Advisory Locks" SYSTEM_COLUMNS_NAME = "System Columns" XML_FUNCTIONS_NAME = "XML Functions" - LIMIT_WITH_TIES = "LIMIT_WITH_TIES" + FETCH_WITH_TIES = "FETCH_WITH_TIES" REGEX_FUNCTIONS = "REGEX_FUNCTIONS" ) diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index 7b821c732..256b82ba5 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -226,7 +226,8 @@ func (d *SelectStmtDetector) Detect(msg protoreflect.Message) error { if err != nil { return err } - // checks if a SelectStmt node uses a LIMIT clause with TIES + // checks if a SelectStmt node uses a FETCH clause with TIES + // https://www.postgresql.org/docs/13/sql-select.html#SQL-LIMIT if selectStmtNode.LimitOption == pg_query.LimitOption_LIMIT_OPTION_WITH_TIES { d.limitOptionWithTiesDetected = true } @@ -237,7 +238,7 @@ func (d *SelectStmtDetector) Detect(msg protoreflect.Message) error { func (d *SelectStmtDetector) GetIssues() []QueryIssue { var issues []QueryIssue if d.limitOptionWithTiesDetected { - issues = append(issues, NewLimitWithTiesIssue(DML_QUERY_OBJECT_TYPE, "", d.query)) + issues = append(issues, NewFetchWithTiesIssue(DML_QUERY_OBJECT_TYPE, "", d.query)) } return issues } diff --git a/yb-voyager/src/query/queryissue/issues_dml.go b/yb-voyager/src/query/queryissue/issues_dml.go index 0f40a5512..6719e7590 100644 --- a/yb-voyager/src/query/queryissue/issues_dml.go +++ b/yb-voyager/src/query/queryissue/issues_dml.go @@ -83,15 +83,15 @@ func NewLOFuntionsIssue(objectType string, objectName string, sqlStatement strin return newQueryIssue(loFunctionsIssue, objectType, objectName, sqlStatement, map[string]interface{}{}) } -var limitWithTiesIssue = issue.Issue{ - Type: LIMIT_WITH_TIES, - TypeName: "LIMIT .. WITH TIES", - TypeDescription: "LIMIT WITH TIES is not supported in YugabyteDB", - Suggestion: "LIMIT WITH TIES is not yet supported in YugabyteDB, no workaround available right now", +var fetchWithTiesIssue = issue.Issue{ + Type: FETCH_WITH_TIES, + TypeName: "FETCH .. WITH TIES", + TypeDescription: "FETCH .. WITH TIES is not supported in YugabyteDB", + Suggestion: "No workaround available right now", GH: "", DocsLink: "", //TODO } -func NewLimitWithTiesIssue(objectType string, objectName string, sqlStatement string) QueryIssue { - return newQueryIssue(limitWithTiesIssue, objectType, objectName, sqlStatement, map[string]interface{}{}) +func NewFetchWithTiesIssue(objectType string, objectName string, sqlStatement string) QueryIssue { + return newQueryIssue(fetchWithTiesIssue, objectType, objectName, sqlStatement, map[string]interface{}{}) } diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go index 2cbc0a3d6..ab21bdb67 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -540,11 +540,11 @@ func TestWithTies(t *testing.T) { ) AS top_employees;` expectedIssues := map[string][]QueryIssue{ - stmt1: []QueryIssue{NewLimitWithTiesIssue("DML_QUERY", "", stmt1)}, - stmt2: []QueryIssue{NewLimitWithTiesIssue("DML_QUERY", "", stmt2)}, + stmt1: []QueryIssue{NewFetchWithTiesIssue("DML_QUERY", "", stmt1)}, + stmt2: []QueryIssue{NewFetchWithTiesIssue("DML_QUERY", "", stmt2)}, } expectedDDLIssues := map[string][]QueryIssue{ - stmt3: []QueryIssue{NewLimitWithTiesIssue("VIEW", "top_employees_view", stmt3)}, + stmt3: []QueryIssue{NewFetchWithTiesIssue("VIEW", "top_employees_view", stmt3)}, } parserIssueDetector := NewParserIssueDetector() From f999e0b85db1a3c6f39cd3c13bea2c478778cf66 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Mon, 23 Dec 2024 16:23:31 +0530 Subject: [PATCH 06/18] issue test --- .../src/query/queryissue/issues_dml_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/yb-voyager/src/query/queryissue/issues_dml_test.go b/yb-voyager/src/query/queryissue/issues_dml_test.go index af3d8afcc..6e874ea25 100644 --- a/yb-voyager/src/query/queryissue/issues_dml_test.go +++ b/yb-voyager/src/query/queryissue/issues_dml_test.go @@ -59,6 +59,25 @@ func testRegexFunctionsIssue(t *testing.T) { } } +func testFetchWithTiesIssue(t *testing.T) { + ctx := context.Background() + conn, err := getConn() + assert.NoError(t, err) + + defer conn.Close(context.Background()) + + stmts := []string{ + `SELECT * FROM employees + ORDER BY salary DESC + FETCH FIRST 2 ROWS WITH TIES;`, + } + + for _, stmt := range stmts { + _, err = conn.Exec(ctx, stmt) + assertErrorCorrectlyThrownForIssueForYBVersion(t, err, `syntax error at or near "WITH"`, regexFunctionsIssue) + } +} + func TestDMLIssuesInYBVersion(t *testing.T) { var err error ybVersion := os.Getenv("YB_VERSION") @@ -90,4 +109,7 @@ func TestDMLIssuesInYBVersion(t *testing.T) { success = t.Run(fmt.Sprintf("%s-%s", "regex functions", ybVersion), testRegexFunctionsIssue) assert.True(t, success) + success = t.Run(fmt.Sprintf("%s-%s", "fetch with ties", ybVersion), testFetchWithTiesIssue) + assert.True(t, success) + } From 87637ee5693873516768617480b77217394ac48d Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Tue, 24 Dec 2024 11:07:14 +0530 Subject: [PATCH 07/18] assess-migration support --- yb-voyager/cmd/assessMigrationCommand.go | 1 + yb-voyager/cmd/constants.go | 1 + 2 files changed, 2 insertions(+) diff --git a/yb-voyager/cmd/assessMigrationCommand.go b/yb-voyager/cmd/assessMigrationCommand.go index f67697ae1..fea1f9b45 100644 --- a/yb-voyager/cmd/assessMigrationCommand.go +++ b/yb-voyager/cmd/assessMigrationCommand.go @@ -1014,6 +1014,7 @@ func fetchUnsupportedPGFeaturesFromSchemaReport(schemaAnalysisReport utils.Schem unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.SYSTEM_COLUMNS_NAME, queryissue.SYSTEM_COLUMNS_NAME, "", schemaAnalysisReport, false, "")) unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.LARGE_OBJECT_FUNCTIONS_NAME, queryissue.LARGE_OBJECT_FUNCTIONS_NAME, "", schemaAnalysisReport, false, "")) unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(REGEX_FUNCTIONS_FEATURE, "", queryissue.REGEX_FUNCTIONS, schemaAnalysisReport, false, "")) + unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(FETCH_WITH_TIES_FEATURE, "", queryissue.FETCH_WITH_TIES, schemaAnalysisReport, false, "")) return lo.Filter(unsupportedFeatures, func(f UnsupportedFeature, _ int) bool { return len(f.Objects) > 0 diff --git a/yb-voyager/cmd/constants.go b/yb-voyager/cmd/constants.go index b22891ae0..9b671709e 100644 --- a/yb-voyager/cmd/constants.go +++ b/yb-voyager/cmd/constants.go @@ -214,6 +214,7 @@ const ( BEFORE_FOR_EACH_ROW_TRIGGERS_ON_PARTITIONED_TABLE_FEATURE = "BEFORE ROW triggers on Partitioned tables" PK_UK_CONSTRAINT_ON_COMPLEX_DATATYPES_FEATURE = "Primary / Unique key constraints on complex datatypes" REGEX_FUNCTIONS_FEATURE = "Regex Functions" + FETCH_WITH_TIES_FEATURE = "FETCH .. WITH TIES" // Migration caveats From 8336ad3a75785340591ca90cf60b4aa9f2934dac Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Tue, 24 Dec 2024 11:15:29 +0530 Subject: [PATCH 08/18] analyze-schema test --- .../dummy-export-dir/schema/tables/table.sql | 4 +++- .../dummy-export-dir/schema/views/view.sql | 8 +++++++- migtests/tests/analyze-schema/expected_issues.json | 10 ++++++++++ migtests/tests/analyze-schema/summary.json | 10 +++++----- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/migtests/tests/analyze-schema/dummy-export-dir/schema/tables/table.sql b/migtests/tests/analyze-schema/dummy-export-dir/schema/tables/table.sql index 82f7411d3..05dfbf7dc 100755 --- a/migtests/tests/analyze-schema/dummy-export-dir/schema/tables/table.sql +++ b/migtests/tests/analyze-schema/dummy-export-dir/schema/tables/table.sql @@ -384,4 +384,6 @@ CREATE TABLE public.locations ( created_at TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP ); -CREATE TABLE image (title text, raster lo); \ No newline at end of file +CREATE TABLE image (title text, raster lo); + +CREATE TABLE employees (id INT PRIMARY KEY, salary INT); \ No newline at end of file diff --git a/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql b/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql index 435adb925..ef7278a01 100644 --- a/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql +++ b/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql @@ -32,4 +32,10 @@ CREATE VIEW public.orders_view AS pg_try_advisory_lock((hashtext((orders.customer_name || orders.product_name)))::bigint) AS lock_acquired, orders.ctid AS row_ctid, orders.xmin AS transaction_id - FROM public.orders; \ No newline at end of file + FROM public.orders; + +CREATE VIEW top_employees_view AS SELECT * FROM ( + SELECT * FROM employees + ORDER BY salary DESC + FETCH FIRST 2 ROWS WITH TIES + ) AS top_employees; \ No newline at end of file diff --git a/migtests/tests/analyze-schema/expected_issues.json b/migtests/tests/analyze-schema/expected_issues.json index 35cc21ec2..5542b51f9 100644 --- a/migtests/tests/analyze-schema/expected_issues.json +++ b/migtests/tests/analyze-schema/expected_issues.json @@ -1900,5 +1900,15 @@ "Suggestion": "Large objects are not yet supported in YugabyteDB, no workaround available currently", "GH": "https://github.com/yugabyte/yugabyte-db/issues/25318", "MinimumVersionsFixedIn": null + }, + { + "IssueType": "unsupported_features", + "ObjectType": "VIEW", + "ObjectName": "top_employees_view", + "Reason": "FETCH .. WITH TIES", + "SqlStatement": "CREATE VIEW top_employees_view AS SELECT * FROM (\n\t\t\tSELECT * FROM employees\n\t\t\tORDER BY salary DESC\n\t\t\tFETCH FIRST 2 ROWS WITH TIES\n\t\t) AS top_employees;", + "Suggestion": "No workaround available right now", + "GH": "", + "MinimumVersionsFixedIn": null } ] diff --git a/migtests/tests/analyze-schema/summary.json b/migtests/tests/analyze-schema/summary.json index 969e87da8..095269371 100644 --- a/migtests/tests/analyze-schema/summary.json +++ b/migtests/tests/analyze-schema/summary.json @@ -26,9 +26,9 @@ }, { "ObjectType": "TABLE", - "TotalCount": 51, + "TotalCount": 52, "InvalidCount": 43, - "ObjectNames": "image, public.xml_data_example, combined_tbl1, test_arr_enum, public.locations, test_udt, combined_tbl, public.ts_query_table, public.documents, public.citext_type, public.inet_type, public.test_jsonb, test_xml_type, test_xid_type, public.range_columns_partition_test_copy, anydata_test, uritype_test, public.foreign_def_test, test_4, enum_example.bugs, table_abc, anydataset_test, unique_def_test1, test_2, table_1, public.range_columns_partition_test, table_xyz, public.users, test_3, test_5, test_7, foreign_def_test2, unique_def_test, sales_data, table_test, test_interval, test_non_pk_multi_column_list, test_9, test_8, order_details, public.employees4, anytype_test, public.meeting, test_table_in_type_file, sales, test_1, \"Test\", foreign_def_test1, salaries2, test_6, public.pr" }, + "ObjectNames": "employees, image, public.xml_data_example, combined_tbl1, test_arr_enum, public.locations, test_udt, combined_tbl, public.ts_query_table, public.documents, public.citext_type, public.inet_type, public.test_jsonb, test_xml_type, test_xid_type, public.range_columns_partition_test_copy, anydata_test, uritype_test, public.foreign_def_test, test_4, enum_example.bugs, table_abc, anydataset_test, unique_def_test1, test_2, table_1, public.range_columns_partition_test, table_xyz, public.users, test_3, test_5, test_7, foreign_def_test2, unique_def_test, sales_data, table_test, test_interval, test_non_pk_multi_column_list, test_9, test_8, order_details, public.employees4, anytype_test, public.meeting, test_table_in_type_file, sales, test_1, \"Test\", foreign_def_test1, salaries2, test_6, public.pr" }, { "ObjectType": "INDEX", "TotalCount": 43, @@ -50,9 +50,9 @@ }, { "ObjectType": "VIEW", - "TotalCount": 5, - "InvalidCount": 5, - "ObjectNames": "v1, v2, test, public.orders_view, view_name" + "TotalCount": 6, + "InvalidCount": 6, + "ObjectNames": "v1, v2, test, public.orders_view, view_name, top_employees_view" }, { "ObjectType": "TRIGGER", From 6981662ab223a095dcc78bfeff7d77b7b23c2fef Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Tue, 24 Dec 2024 11:26:56 +0530 Subject: [PATCH 09/18] assessment test --- .../expectedAssessmentReport.json | 56 ++++++++++++++++--- .../pg_assessment_report.sql | 14 +++++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index edfe82ecd..735434970 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -38,15 +38,15 @@ }, { "ObjectType": "SEQUENCE", - "TotalCount": 27, + "TotalCount": 29, "InvalidCount": 0, - "ObjectNames": "public.ordersentry_order_id_seq, public.\"Case_Sensitive_Columns_id_seq\", public.\"Mixed_Case_Table_Name_Test_id_seq\", public.\"Recipients_id_seq\", public.\"WITH_id_seq\", public.employees2_id_seq, public.ext_test_id_seq, public.mixed_data_types_table1_id_seq, public.mixed_data_types_table2_id_seq, public.orders2_id_seq, public.parent_table_id_seq, public.with_example1_id_seq, public.with_example2_id_seq, schema2.\"Case_Sensitive_Columns_id_seq\", schema2.\"Mixed_Case_Table_Name_Test_id_seq\", schema2.\"Recipients_id_seq\", schema2.\"WITH_id_seq\", schema2.employees2_id_seq, schema2.ext_test_id_seq, schema2.mixed_data_types_table1_id_seq, schema2.mixed_data_types_table2_id_seq, schema2.orders2_id_seq, schema2.parent_table_id_seq, schema2.with_example1_id_seq, schema2.with_example2_id_seq, test_views.view_table1_id_seq, test_views.view_table2_id_seq" + "ObjectNames": "public.ordersentry_order_id_seq, public.\"Case_Sensitive_Columns_id_seq\", public.\"Mixed_Case_Table_Name_Test_id_seq\", public.\"Recipients_id_seq\", public.\"WITH_id_seq\", public.employees2_id_seq, public.ext_test_id_seq, public.mixed_data_types_table1_id_seq, public.mixed_data_types_table2_id_seq, public.orders2_id_seq, public.parent_table_id_seq, public.with_example1_id_seq, public.with_example2_id_seq, schema2.\"Case_Sensitive_Columns_id_seq\", schema2.\"Mixed_Case_Table_Name_Test_id_seq\", schema2.\"Recipients_id_seq\", schema2.\"WITH_id_seq\", schema2.employees2_id_seq, schema2.ext_test_id_seq, schema2.mixed_data_types_table1_id_seq, schema2.mixed_data_types_table2_id_seq, schema2.orders2_id_seq, schema2.parent_table_id_seq, schema2.with_example1_id_seq, schema2.with_example2_id_seq, test_views.view_table1_id_seq, test_views.view_table2_id_seq, public.employees_id_seq, schema2.employees_id_seq" }, { "ObjectType": "TABLE", - "TotalCount": 66, + "TotalCount": 68, "InvalidCount": 23, - "ObjectNames": "public.ordersentry, public.library_nested, public.orders_lateral, public.\"Case_Sensitive_Columns\", public.\"Mixed_Case_Table_Name_Test\", public.\"Recipients\", public.\"WITH\", public.audit, public.sales_region, public.boston, public.c, public.parent_table, public.child_table, public.citext_type, public.combined_tbl, public.documents, public.employees2, public.ext_test, public.foo, public.inet_type, public.london, public.mixed_data_types_table1, public.mixed_data_types_table2, public.orders, public.orders2, public.products, public.session_log, public.session_log1, public.session_log2, public.sydney, public.test_exclude_basic, public.test_jsonb, public.test_xml_type, public.ts_query_table, public.tt, public.with_example1, public.with_example2, schema2.\"Case_Sensitive_Columns\", schema2.\"Mixed_Case_Table_Name_Test\", schema2.\"Recipients\", schema2.\"WITH\", schema2.audit, schema2.sales_region, schema2.boston, schema2.c, schema2.parent_table, schema2.child_table, schema2.employees2, schema2.ext_test, schema2.foo, schema2.london, schema2.mixed_data_types_table1, schema2.mixed_data_types_table2, schema2.orders, schema2.orders2, schema2.products, schema2.session_log, schema2.session_log1, schema2.session_log2, schema2.sydney, schema2.test_xml_type, schema2.tt, schema2.with_example1, schema2.with_example2, test_views.view_table1, test_views.view_table2" + "ObjectNames": "public.ordersentry, public.library_nested, public.orders_lateral, public.\"Case_Sensitive_Columns\", public.\"Mixed_Case_Table_Name_Test\", public.\"Recipients\", public.\"WITH\", public.audit, public.sales_region, public.boston, public.c, public.parent_table, public.child_table, public.citext_type, public.combined_tbl, public.documents, public.employees2, public.ext_test, public.foo, public.inet_type, public.london, public.mixed_data_types_table1, public.mixed_data_types_table2, public.orders, public.orders2, public.products, public.session_log, public.session_log1, public.session_log2, public.sydney, public.test_exclude_basic, public.test_jsonb, public.test_xml_type, public.ts_query_table, public.tt, public.with_example1, public.with_example2, schema2.\"Case_Sensitive_Columns\", schema2.\"Mixed_Case_Table_Name_Test\", schema2.\"Recipients\", schema2.\"WITH\", schema2.audit, schema2.sales_region, schema2.boston, schema2.c, schema2.parent_table, schema2.child_table, schema2.employees2, schema2.ext_test, schema2.foo, schema2.london, schema2.mixed_data_types_table1, schema2.mixed_data_types_table2, schema2.orders, schema2.orders2, schema2.products, schema2.session_log, schema2.session_log1, schema2.session_log2, schema2.sydney, schema2.test_xml_type, schema2.tt, schema2.with_example1, schema2.with_example2, test_views.view_table1, test_views.view_table2, public.employees, schema2.employees" }, { "ObjectType": "INDEX", @@ -73,9 +73,9 @@ }, { "ObjectType": "VIEW", - "TotalCount": 7, - "InvalidCount": 3, - "ObjectNames": "public.ordersentry_view, public.sales_employees, schema2.sales_employees, test_views.v1, test_views.v2, test_views.v3, test_views.v4" + "TotalCount": 9, + "InvalidCount": 5, + "ObjectNames": "public.ordersentry_view, public.sales_employees, schema2.sales_employees, test_views.v1, test_views.v2, test_views.v3, test_views.v4, schema2.top_employees_view, public.top_employees_view" }, { "ObjectType": "TRIGGER", @@ -555,6 +555,20 @@ } ], "MinimumVersionsFixedIn": null + }, + { + "FeatureName": "FETCH .. WITH TIES", + "Objects": [ + { + "ObjectName": "public.top_employees_view", + "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employees\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + }, + { + "ObjectName": "schema2.top_employees_view", + "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employees\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + } + ], + "MinimumVersionsFixedIn": null } ], "UnsupportedFeaturesDesc": "Features of the source database that are not supported on the target YugabyteDB.", @@ -1958,6 +1972,34 @@ "ObjectType": "", "ParentTableName": "schema2.mixed_data_types_table1", "SizeInBytes": 8192 + }, + { + "SchemaName": "public", + "ObjectName": "employees", + "RowCount": 0, + "ColumnCount": 4, + "Reads": 0, + "Writes": 0, + "ReadsPerSecond": 0, + "WritesPerSecond": 0, + "IsIndex": false, + "ObjectType": "", + "ParentTableName": null, + "SizeInBytes": 0 + }, + { + "SchemaName": "schema2", + "ObjectName": "employees", + "RowCount": 0, + "ColumnCount": 4, + "Reads": 0, + "Writes": 0, + "ReadsPerSecond": 0, + "WritesPerSecond": 0, + "IsIndex": false, + "ObjectType": "", + "ParentTableName": null, + "SizeInBytes": 0 } ], "Notes": [ diff --git a/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql b/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql index 0c089922b..fdd9dfbc5 100644 --- a/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql +++ b/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql @@ -362,3 +362,17 @@ BEGIN END IF; END; $$ LANGUAGE plpgsql; + +-- for FETCH .. WITH TIES +CREATE TABLE employees ( + id SERIAL PRIMARY KEY, + first_name VARCHAR(50) NOT NULL, + last_name VARCHAR(50) NOT NULL, + salary NUMERIC(10, 2) NOT NULL +); + +CREATE VIEW top_employees_view AS SELECT * FROM ( + SELECT * FROM employees + ORDER BY salary DESC + FETCH FIRST 2 ROWS WITH TIES + ) AS top_employees; \ No newline at end of file From 0037cea4f4466fc6f6b624a7b6513bb52cd21e42 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 15:15:16 +0530 Subject: [PATCH 10/18] review comments --- yb-voyager/cmd/constants.go | 2 +- yb-voyager/src/query/queryissue/detectors.go | 17 ++--------------- .../queryissue/parser_issue_detector_test.go | 2 +- .../src/query/queryparser/helpers_protomsg.go | 11 +++++++++++ .../src/query/queryparser/helpers_struct.go | 4 ++++ 5 files changed, 19 insertions(+), 17 deletions(-) diff --git a/yb-voyager/cmd/constants.go b/yb-voyager/cmd/constants.go index 9b671709e..28f7e5c77 100644 --- a/yb-voyager/cmd/constants.go +++ b/yb-voyager/cmd/constants.go @@ -214,7 +214,7 @@ const ( BEFORE_FOR_EACH_ROW_TRIGGERS_ON_PARTITIONED_TABLE_FEATURE = "BEFORE ROW triggers on Partitioned tables" PK_UK_CONSTRAINT_ON_COMPLEX_DATATYPES_FEATURE = "Primary / Unique key constraints on complex datatypes" REGEX_FUNCTIONS_FEATURE = "Regex Functions" - FETCH_WITH_TIES_FEATURE = "FETCH .. WITH TIES" + FETCH_WITH_TIES_FEATURE = "FETCH .. WITH TIES Clause" // Migration caveats diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index 256b82ba5..15c2efb1e 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -16,12 +16,8 @@ limitations under the License. package queryissue import ( - "fmt" - mapset "github.com/deckarep/golang-set/v2" - 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" "github.com/yugabyte/yb-voyager/yb-voyager/src/query/queryparser" @@ -211,24 +207,15 @@ func NewSelectStmtDetector(query string) *SelectStmtDetector { } } -func (d *SelectStmtDetector) getSelectStmtFromProto(msg protoreflect.Message) (*pg_query.SelectStmt, error) { - protoMsg := msg.Interface().(proto.Message) - selectStmtNode, ok := protoMsg.(*pg_query.SelectStmt) - if !ok { - return nil, fmt.Errorf("failed to cast %s to %s", queryparser.PG_QUERY_SELECTSTMT_NODE, queryparser.PG_QUERY_SELECTSTMT_NODE) - } - return selectStmtNode, nil -} - func (d *SelectStmtDetector) Detect(msg protoreflect.Message) error { if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_SELECTSTMT_NODE { - selectStmtNode, err := d.getSelectStmtFromProto(msg) + selectStmtNode, err := queryparser.ProtoAsSelectStmt(msg) if err != nil { return err } // checks if a SelectStmt node uses a FETCH clause with TIES // https://www.postgresql.org/docs/13/sql-select.html#SQL-LIMIT - if selectStmtNode.LimitOption == pg_query.LimitOption_LIMIT_OPTION_WITH_TIES { + if selectStmtNode.LimitOption == queryparser.LIMIT_OPTION_WITH_TIES { d.limitOptionWithTiesDetected = true } } diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go index ab21bdb67..e6961fffb 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -516,7 +516,7 @@ func TestRegexFunctionsIssue(t *testing.T) { } -func TestWithTies(t *testing.T) { +func TestFetchWithTiesInSelect(t *testing.T) { stmt1 := ` SELECT * FROM employees diff --git a/yb-voyager/src/query/queryparser/helpers_protomsg.go b/yb-voyager/src/query/queryparser/helpers_protomsg.go index 0eb609d15..28d57d52d 100644 --- a/yb-voyager/src/query/queryparser/helpers_protomsg.go +++ b/yb-voyager/src/query/queryparser/helpers_protomsg.go @@ -16,10 +16,12 @@ limitations under the License. package queryparser import ( + "fmt" "strings" 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" ) @@ -370,3 +372,12 @@ func GetSchemaAndObjectName(nameList protoreflect.List) (string, string) { } return schemaName, objectName } + +func ProtoAsSelectStmt(msg protoreflect.Message) (*pg_query.SelectStmt, error) { + protoMsg := msg.Interface().(proto.Message) + selectStmtNode, ok := protoMsg.(*pg_query.SelectStmt) + if !ok { + return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_SELECTSTMT_NODE) + } + return selectStmtNode, nil +} diff --git a/yb-voyager/src/query/queryparser/helpers_struct.go b/yb-voyager/src/query/queryparser/helpers_struct.go index eafc90791..37d421bdf 100644 --- a/yb-voyager/src/query/queryparser/helpers_struct.go +++ b/yb-voyager/src/query/queryparser/helpers_struct.go @@ -23,6 +23,10 @@ import ( "github.com/samber/lo" ) +const ( + LIMIT_OPTION_WITH_TIES = pg_query.LimitOption_LIMIT_OPTION_WITH_TIES +) + func IsPLPGSQLObject(parseTree *pg_query.ParseResult) bool { // CREATE FUNCTION is same parser NODE for FUNCTION/PROCEDURE _, isPlPgSQLObject := getCreateFuncStmtNode(parseTree) From 2d8eec28e7f5cfc6561ada9a645dd2a091271301 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 15:18:53 +0530 Subject: [PATCH 11/18] fix test --- .../expectedAssessmentReport.json | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index 735434970..e16e8c848 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -162,6 +162,7 @@ "schema2.products", "schema2.foo", "schema2.Case_Sensitive_Columns", + "schema2.employees", "schema2.with_example1", "test_views.xyz_mview", "test_views.view_table2", @@ -169,9 +170,10 @@ "test_views.abc_mview", "test_views.view_table1", "public.library_nested", - "public.orders_lateral" + "public.orders_lateral", + "public.employees" ], - "ColocatedReasoning": "Recommended instance type with 4 vCPU and 16 GiB memory could fit 72 objects (64 tables/materialized views and 8 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec as colocated. Rest 28 objects (5 tables/materialized views and 23 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec need to be migrated as range partitioned tables. Non leaf partition tables/indexes and unsupported tables/indexes were not considered.", + "ColocatedReasoning": "Recommended instance type with 4 vCPU and 16 GiB memory could fit 74 objects (66 tables/materialized views and 8 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec as colocated. Rest 28 objects (5 tables/materialized views and 23 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec need to be migrated as range partitioned tables. Non leaf partition tables/indexes and unsupported tables/indexes were not considered.", "ShardedTables": [ "public.combined_tbl", "public.citext_type", @@ -561,11 +563,11 @@ "Objects": [ { "ObjectName": "public.top_employees_view", - "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employees\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employees\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" }, { "ObjectName": "schema2.top_employees_view", - "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employees\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employees\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" } ], "MinimumVersionsFixedIn": null From 0d8b01f46590b8dbadebec1fdcca3af5a0ca6dae Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 15:42:51 +0530 Subject: [PATCH 12/18] missed merge conflict --- .../src/query/queryissue/parser_issue_detector_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go index f2d57143a..e5b6315b0 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -452,19 +452,11 @@ $$ LANGUAGE plpgsql; NewLOFuntionsIssue("TRIGGER", "t_raster ON image", sqls[5], []string{"lo_manage"}), }, } -<<<<<<< HEAD - expectedSQLsWithIssues[sqls[0]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[0]], "FUNCTION", "manage_large_object") - expectedSQLsWithIssues[sqls[1]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[1]], "FUNCTION", "import_file_to_table") - expectedSQLsWithIssues[sqls[2]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[2]], "FUNCTION", "export_large_object") - expectedSQLsWithIssues[sqls[3]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[3]], "PROCEDURE", "read_large_object") - expectedSQLsWithIssues[sqls[4]] = modifyiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[4]], "FUNCTION", "write_to_large_object") -======= expectedSQLsWithIssues[sqls[0]] = modifiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[0]], "FUNCTION", "manage_large_object") expectedSQLsWithIssues[sqls[1]] = modifiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[1]], "FUNCTION", "import_file_to_table") expectedSQLsWithIssues[sqls[2]] = modifiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[2]], "FUNCTION", "export_large_object") expectedSQLsWithIssues[sqls[3]] = modifiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[3]], "PROCEDURE", "read_large_object") expectedSQLsWithIssues[sqls[4]] = modifiedIssuesforPLPGSQL(expectedSQLsWithIssues[sqls[4]], "FUNCTION", "write_to_large_object") ->>>>>>> main parserIssueDetector := NewParserIssueDetector() @@ -661,7 +653,6 @@ func TestRegexFunctionsIssue(t *testing.T) { } - func TestFetchWithTiesInSelect(t *testing.T) { stmt1 := ` @@ -746,7 +737,6 @@ func TestCopyUnsupportedConstructIssuesDetected(t *testing.T) { `COPY my_table FROM '/path/to/data.csv';`: {}, `COPY my_table FROM '/path/to/data.csv' WITH (DELIMITER ',');`: {}, `COPY my_table(col1, col2) FROM '/path/to/data.csv' WITH (FORMAT csv, HEADER true);`: {}, - } parserIssueDetector := NewParserIssueDetector() From 0274a38f5e5b8a8e194c61d98cae350c2fdcd1ed Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 15:52:58 +0530 Subject: [PATCH 13/18] go mod tidy --- yb-voyager/go.mod | 4 ---- yb-voyager/go.sum | 6 +----- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/yb-voyager/go.mod b/yb-voyager/go.mod index 5bb9e6cb6..7df52aa40 100644 --- a/yb-voyager/go.mod +++ b/yb-voyager/go.mod @@ -15,7 +15,6 @@ require ( github.com/docker/go-connections v0.5.0 github.com/dustin/go-humanize v1.0.1 github.com/fatih/color v1.13.0 - github.com/fergusstrange/embedded-postgres v1.29.0 github.com/go-sql-driver/mysql v1.7.0 github.com/godror/godror v0.30.2 github.com/google/uuid v1.6.0 @@ -25,7 +24,6 @@ require ( github.com/jackc/pgconn v1.14.3 github.com/jackc/pgx/v4 v4.18.3 github.com/jackc/pgx/v5 v5.0.3 - github.com/lib/pq v1.10.9 github.com/mattn/go-sqlite3 v1.14.17 github.com/mcuadros/go-version v0.0.0-20190830083331-035f6764e8d2 github.com/mitchellh/go-ps v1.0.0 @@ -45,7 +43,6 @@ require ( golang.org/x/term v0.24.0 google.golang.org/api v0.169.0 gopkg.in/natefinch/lumberjack.v2 v2.2.1 - gotest.tools/v3 v3.5.1 gotest.tools v2.2.0+incompatible ) @@ -84,7 +81,6 @@ require ( github.com/shoenig/go-m1cpu v0.1.6 // indirect github.com/tklauser/go-sysconf v0.3.12 // indirect github.com/tklauser/numcpus v0.6.1 // indirect - github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect github.com/yusufpapurcu/wmi v1.2.3 // indirect go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect diff --git a/yb-voyager/go.sum b/yb-voyager/go.sum index bedf547e1..1c06c76f9 100644 --- a/yb-voyager/go.sum +++ b/yb-voyager/go.sum @@ -919,8 +919,6 @@ github.com/felixge/httpsnoop v1.0.2/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSw github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/fergusstrange/embedded-postgres v1.29.0 h1:Uv8hdhoiaNMuH0w8UuGXDHr60VoAQPFdgx7Qf3bzXJM= -github.com/fergusstrange/embedded-postgres v1.29.0/go.mod h1:t/MLs0h9ukYM6FSt99R7InCHs1nW0ordoVCcnzmpTYw= github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k= github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= github.com/form3tech-oss/jwt-go v3.2.3+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= @@ -1971,8 +1969,6 @@ github.com/xdg-go/stringprep v1.0.3/go.mod h1:W3f5j4i+9rC0kuIEJL0ky1VpHXQU3ocBgk github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f/go.mod h1:5yf86TLmAcydyeJq5YvxkGPE2fm/u4myDekKRoLuqhs= -github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 h1:nIPpBwaJSVYIxUFsDv3M8ofmx9yWTog9BfvIu0q41lo= -github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8/go.mod h1:HUYIGzjTL3rfEspMxjDjgmT5uz5wzYJKVo23qUhYTos= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/xlab/treeprint v1.1.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= @@ -2103,7 +2099,6 @@ go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A go.uber.org/goleak v1.1.11-0.20210813005559-691160354723/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= -go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= @@ -2991,6 +2986,7 @@ gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= From 5df9e184ce2f574787ffef531d4861ecfd9de434 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 16:02:09 +0530 Subject: [PATCH 14/18] some test fixes --- .../expectedAssessmentReport.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index 59aa4ec9b..b1a67c8de 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -40,13 +40,13 @@ "ObjectType": "SEQUENCE", "TotalCount": 43, "InvalidCount": 0, - "ObjectNames": "public.employees_id_seq, schema2.employees_id_seq, public.\"Case_Sensitive_Columns_id_seq\", public.\"Mixed_Case_Table_Name_Test_id_seq\", public.\"Recipients_id_seq\", public.\"WITH_id_seq\", public.bigint_multirange_table_id_seq, public.date_multirange_table_id_seq, public.employees2_id_seq, public.employees3_id_seq, public.employees_employee_id_seq, public.ext_test_id_seq, public.int_multirange_table_id_seq, public.mixed_data_types_table1_id_seq, public.mixed_data_types_table2_id_seq, public.numeric_multirange_table_id_seq, public.orders2_id_seq, public.ordersentry_order_id_seq, public.parent_table_id_seq, public.timestamp_multirange_table_id_seq, public.timestamptz_multirange_table_id_seq, public.with_example1_id_seq, public.with_example2_id_seq, schema2.\"Case_Sensitive_Columns_id_seq\", schema2.\"Mixed_Case_Table_Name_Test_id_seq\", schema2.\"Recipients_id_seq\", schema2.\"WITH_id_seq\", schema2.bigint_multirange_table_id_seq, schema2.date_multirange_table_id_seq, schema2.employees2_id_seq, schema2.ext_test_id_seq, schema2.int_multirange_table_id_seq, schema2.mixed_data_types_table1_id_seq, schema2.mixed_data_types_table2_id_seq, schema2.numeric_multirange_table_id_seq, schema2.orders2_id_seq, schema2.parent_table_id_seq, schema2.timestamp_multirange_table_id_seq, schema2.timestamptz_multirange_table_id_seq, schema2.with_example1_id_seq, schema2.with_example2_id_seq, test_views.view_table1_id_seq, test_views.view_table2_id_seq" + "ObjectNames": "public.employeesforview_id_seq, schema2.employeesforview_id_seq, public.\"Case_Sensitive_Columns_id_seq\", public.\"Mixed_Case_Table_Name_Test_id_seq\", public.\"Recipients_id_seq\", public.\"WITH_id_seq\", public.bigint_multirange_table_id_seq, public.date_multirange_table_id_seq, public.employees2_id_seq, public.employees3_id_seq, public.employees_employee_id_seq, public.ext_test_id_seq, public.int_multirange_table_id_seq, public.mixed_data_types_table1_id_seq, public.mixed_data_types_table2_id_seq, public.numeric_multirange_table_id_seq, public.orders2_id_seq, public.ordersentry_order_id_seq, public.parent_table_id_seq, public.timestamp_multirange_table_id_seq, public.timestamptz_multirange_table_id_seq, public.with_example1_id_seq, public.with_example2_id_seq, schema2.\"Case_Sensitive_Columns_id_seq\", schema2.\"Mixed_Case_Table_Name_Test_id_seq\", schema2.\"Recipients_id_seq\", schema2.\"WITH_id_seq\", schema2.bigint_multirange_table_id_seq, schema2.date_multirange_table_id_seq, schema2.employees2_id_seq, schema2.ext_test_id_seq, schema2.int_multirange_table_id_seq, schema2.mixed_data_types_table1_id_seq, schema2.mixed_data_types_table2_id_seq, schema2.numeric_multirange_table_id_seq, schema2.orders2_id_seq, schema2.parent_table_id_seq, schema2.timestamp_multirange_table_id_seq, schema2.timestamptz_multirange_table_id_seq, schema2.with_example1_id_seq, schema2.with_example2_id_seq, test_views.view_table1_id_seq, test_views.view_table2_id_seq" }, { "ObjectType": "TABLE", "TotalCount": 82, "InvalidCount": 35, - "ObjectNames": "public.employees, schema2.employees, public.\"Case_Sensitive_Columns\", public.\"Mixed_Case_Table_Name_Test\", public.\"Recipients\", public.\"WITH\", public.audit, public.bigint_multirange_table, public.boston, public.c, public.child_table, public.citext_type, public.combined_tbl, public.date_multirange_table, public.documents, public.employees, public.employees2, public.employees3, public.ext_test, public.foo, public.inet_type, public.int_multirange_table, public.library_nested, public.london, public.mixed_data_types_table1, public.mixed_data_types_table2, public.numeric_multirange_table, public.orders, public.orders2, public.orders_lateral, public.ordersentry, public.parent_table, public.products, public.sales_region, public.session_log, public.session_log1, public.session_log2, public.sydney, public.test_exclude_basic, public.test_jsonb, public.test_xml_type, public.timestamp_multirange_table, public.timestamptz_multirange_table, public.ts_query_table, public.tt, public.with_example1, public.with_example2, schema2.\"Case_Sensitive_Columns\", schema2.\"Mixed_Case_Table_Name_Test\", schema2.\"Recipients\", schema2.\"WITH\", schema2.audit, schema2.bigint_multirange_table, schema2.boston, schema2.c, schema2.child_table, schema2.date_multirange_table, schema2.employees2, schema2.ext_test, schema2.foo, schema2.int_multirange_table, schema2.london, schema2.mixed_data_types_table1, schema2.mixed_data_types_table2, schema2.numeric_multirange_table, schema2.orders, schema2.orders2, schema2.parent_table, schema2.products, schema2.sales_region, schema2.session_log, schema2.session_log1, schema2.session_log2, schema2.sydney, schema2.test_xml_type, schema2.timestamp_multirange_table, schema2.timestamptz_multirange_table, schema2.tt, schema2.with_example1, schema2.with_example2, test_views.view_table1, test_views.view_table2" + "ObjectNames": "public.employeesforview, schema2.employeesforview, public.\"Case_Sensitive_Columns\", public.\"Mixed_Case_Table_Name_Test\", public.\"Recipients\", public.\"WITH\", public.audit, public.bigint_multirange_table, public.boston, public.c, public.child_table, public.citext_type, public.combined_tbl, public.date_multirange_table, public.documents, public.employees, public.employees2, public.employees3, public.ext_test, public.foo, public.inet_type, public.int_multirange_table, public.library_nested, public.london, public.mixed_data_types_table1, public.mixed_data_types_table2, public.numeric_multirange_table, public.orders, public.orders2, public.orders_lateral, public.ordersentry, public.parent_table, public.products, public.sales_region, public.session_log, public.session_log1, public.session_log2, public.sydney, public.test_exclude_basic, public.test_jsonb, public.test_xml_type, public.timestamp_multirange_table, public.timestamptz_multirange_table, public.ts_query_table, public.tt, public.with_example1, public.with_example2, schema2.\"Case_Sensitive_Columns\", schema2.\"Mixed_Case_Table_Name_Test\", schema2.\"Recipients\", schema2.\"WITH\", schema2.audit, schema2.bigint_multirange_table, schema2.boston, schema2.c, schema2.child_table, schema2.date_multirange_table, schema2.employees2, schema2.ext_test, schema2.foo, schema2.int_multirange_table, schema2.london, schema2.mixed_data_types_table1, schema2.mixed_data_types_table2, schema2.numeric_multirange_table, schema2.orders, schema2.orders2, schema2.parent_table, schema2.products, schema2.sales_region, schema2.session_log, schema2.session_log1, schema2.session_log2, schema2.sydney, schema2.test_xml_type, schema2.timestamp_multirange_table, schema2.timestamptz_multirange_table, schema2.tt, schema2.with_example1, schema2.with_example2, test_views.view_table1, test_views.view_table2" }, { "ObjectType": "INDEX", @@ -162,7 +162,7 @@ "schema2.products", "schema2.foo", "schema2.Case_Sensitive_Columns", - "schema2.employeesForView", + "schema2.employeesforview", "schema2.with_example1", "test_views.xyz_mview", "test_views.view_table2", @@ -185,7 +185,7 @@ "schema2.numeric_multirange_table", "schema2.timestamp_multirange_table", "schema2.timestamptz_multirange_table", - "public.employeesForView" + "public.employeesforview" ], "ColocatedReasoning": "Recommended instance type with 4 vCPU and 16 GiB memory could fit 88 objects (80 tables/materialized views and 8 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec as colocated. Rest 28 objects (5 tables/materialized views and 23 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec need to be migrated as range partitioned tables. Non leaf partition tables/indexes and unsupported tables/indexes were not considered.", "ShardedTables": [ @@ -649,11 +649,11 @@ "Objects": [ { "ObjectName": "public.top_employees_view", - "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employeesForView\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" }, { "ObjectName": "schema2.top_employees_view", - "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employeesForView\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" } ], "MinimumVersionsFixedIn": null @@ -2087,7 +2087,7 @@ }, { "SchemaName": "public", - "ObjectName": "employeesForView", + "ObjectName": "employeesforview", "RowCount": 0, "ColumnCount": 4, "Reads": 0, From 83ebcb2454cc9065b82c2f147060773105d57ec5 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 16:04:41 +0530 Subject: [PATCH 15/18] fix createview --- .../tests/pg/assessment-report-test/pg_assessment_report.sql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql b/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql index b01f0b03d..7e3c798ea 100644 --- a/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql +++ b/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql @@ -402,10 +402,11 @@ CREATE TABLE employeesForView ( ); CREATE VIEW top_employees_view AS SELECT * FROM ( - SELECT * FROM employees + SELECT * FROM employeesForView ORDER BY salary DESC FETCH FIRST 2 ROWS WITH TIES ) AS top_employees; + -- SECURITY INVOKER VIEW CREATE TABLE public.employees ( employee_id SERIAL PRIMARY KEY, From d293c2dadf4cb2714cb0c3f4263f5958d3347118 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 16:58:27 +0530 Subject: [PATCH 16/18] Fix test attempt --- .../assessment-report-test/expectedAssessmentReport.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index 05e6ac9e5..4809e030e 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -649,11 +649,11 @@ "Objects": [ { "ObjectName": "public.top_employees_view", - "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" }, { "ObjectName": "schema2.top_employees_view", - "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT top_employees.id,\n top_employees.first_name,\n top_employees.last_name,\n top_employees.salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" } ], "MinimumVersionsFixedIn": null @@ -954,7 +954,7 @@ { "SchemaName": "public", "ObjectName": "employees", - "RowCount": 5, + "RowCount": 10, "ColumnCount": 4, "Reads": 0, "Writes": 10, @@ -2199,7 +2199,7 @@ }, { "SchemaName": "schema2", - "ObjectName": "employees", + "ObjectName": "employeesforview", "RowCount": 0, "ColumnCount": 4, "Reads": 0, From 69c7a4c8ea3b6b112292b1afdd77ccc17267e5cc Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 17:00:18 +0530 Subject: [PATCH 17/18] more fixes --- .../pg/assessment-report-test/expectedAssessmentReport.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index 4809e030e..76e5ffb54 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -649,11 +649,11 @@ "Objects": [ { "ObjectName": "public.top_employees_view", - "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM public.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW public.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employeesforview.id,\n employeesforview.first_name,\n employeesforview.last_name,\n employeesforview.salary\n FROM public.employeesforview\n ORDER BY employeesforview.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" }, { "ObjectName": "schema2.top_employees_view", - "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employees.id,\n employees.first_name,\n employees.last_name,\n employees.salary\n FROM schema2.employeesforview\n ORDER BY employees.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" + "SqlStatement": "CREATE VIEW schema2.top_employees_view AS\n SELECT id,\n first_name,\n last_name,\n salary\n FROM ( SELECT employeesforview.id,\n employeesforview.first_name,\n employeesforview.last_name,\n employeesforview.salary\n FROM schema2.employeesforview\n ORDER BY employeesforview.salary DESC\n FETCH FIRST 2 ROWS WITH TIES) top_employees;" } ], "MinimumVersionsFixedIn": null From 65c787e476efa5381e3b5b57b1c6da4b8b859658 Mon Sep 17 00:00:00 2001 From: Aneesh Makala Date: Fri, 27 Dec 2024 17:40:56 +0530 Subject: [PATCH 18/18] check type assertion --- yb-voyager/src/query/queryparser/helpers_protomsg.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/yb-voyager/src/query/queryparser/helpers_protomsg.go b/yb-voyager/src/query/queryparser/helpers_protomsg.go index 4713a9251..de3204f3c 100644 --- a/yb-voyager/src/query/queryparser/helpers_protomsg.go +++ b/yb-voyager/src/query/queryparser/helpers_protomsg.go @@ -409,7 +409,10 @@ func GetSchemaAndObjectName(nameList protoreflect.List) (string, string) { } func ProtoAsSelectStmt(msg protoreflect.Message) (*pg_query.SelectStmt, error) { - protoMsg := msg.Interface().(proto.Message) + 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) if !ok { return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_SELECTSTMT_NODE)