From b066843e55750b607c802d4a0f53d80b9172e814 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Thu, 2 Jan 2025 18:34:22 +0530 Subject: [PATCH 1/9] Reporting Foreign key references partitioned table issue --- yb-voyager/cmd/assessMigrationCommand.go | 2 +- yb-voyager/src/query/queryissue/constants.go | 3 + yb-voyager/src/query/queryissue/detectors.go | 46 ++++++++++++++++ .../src/query/queryissue/detectors_test.go | 10 ++++ yb-voyager/src/query/queryissue/issues_ddl.go | 12 ++++ .../src/query/queryissue/issues_ddl_test.go | 18 ++++++ .../query/queryissue/parser_issue_detector.go | 1 + .../queryissue/parser_issue_detector_test.go | 55 ++++++++++++++++++- .../src/query/queryparser/helpers_protomsg.go | 12 ++++ .../src/query/queryparser/traversal_proto.go | 1 + 10 files changed, 158 insertions(+), 2 deletions(-) diff --git a/yb-voyager/cmd/assessMigrationCommand.go b/yb-voyager/cmd/assessMigrationCommand.go index c54e998f5..739302a62 100644 --- a/yb-voyager/cmd/assessMigrationCommand.go +++ b/yb-voyager/cmd/assessMigrationCommand.go @@ -1028,7 +1028,7 @@ func fetchUnsupportedPGFeaturesFromSchemaReport(schemaAnalysisReport utils.Schem unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.JSON_CONSTRUCTOR_FUNCTION_NAME, "", queryissue.JSON_CONSTRUCTOR_FUNCTION, schemaAnalysisReport, false, "")) unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.AGGREGATION_FUNCTIONS_NAME, "", queryissue.AGGREGATE_FUNCTION, schemaAnalysisReport, false, "")) unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.SECURITY_INVOKER_VIEWS_NAME, "", queryissue.SECURITY_INVOKER_VIEWS, schemaAnalysisReport, false, "")) - + unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME, "", queryissue.FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, schemaAnalysisReport, false, "")) return lo.Filter(unsupportedFeatures, func(f UnsupportedFeature, _ int) bool { return len(f.Objects) > 0 }), nil diff --git a/yb-voyager/src/query/queryissue/constants.go b/yb-voyager/src/query/queryissue/constants.go index 586957bf9..22c3ff61f 100644 --- a/yb-voyager/src/query/queryissue/constants.go +++ b/yb-voyager/src/query/queryissue/constants.go @@ -74,6 +74,9 @@ const ( MULTI_RANGE_DATATYPE = "MULTI_RANGE_DATATYPE" COPY_FROM_WHERE = "COPY FROM ... WHERE" COPY_ON_ERROR = "COPY ... ON_ERROR" + + FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE = "FOREIGN_KEY_REFERENCED_PARTITIONED_TABLE" + FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME = "Foreign key constraint references partitioned table" ) // Object types diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index c834fa6db..6e2fb60bb 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -19,6 +19,7 @@ import ( "slices" mapset "github.com/deckarep/golang-set/v2" + "github.com/samber/lo" log "github.com/sirupsen/logrus" "google.golang.org/protobuf/reflect/protoreflect" @@ -390,3 +391,48 @@ func (d *JsonQueryFunctionDetector) GetIssues() []QueryIssue { } return issues } + +type ConstraintIssuesDetector struct { + query string + partitionTablesMap map[string]bool + isForeignKeyReferencesPartitionTableDetected bool +} + +func NewConstraintIssuesDetector(query string, partitionTablesMap map[string]bool) *ConstraintIssuesDetector { + return &ConstraintIssuesDetector{ + query: query, + partitionTablesMap: partitionTablesMap, + } +} + +func (c *ConstraintIssuesDetector) Detect(msg protoreflect.Message) error { + if queryparser.GetMsgFullName(msg) != queryparser.PG_QUERY_CONSTRAINT_NODE { + return nil + } + + constraintNode, err := queryparser.GetConstraintNode(msg) + if err != nil { + return err + } + + switch constraintNode.Contype { + case queryparser.FOREIGN_CONSTR_TYPE: + referencedTable := constraintNode.Pktable + //constraints:{constraint:{contype:CONSTR_FOREIGN initially_valid:true pktable:{relname:"abc1" inh:true relpersistence:"p" location:121} + referencedTableObjName := lo.Ternary(referencedTable.Schemaname != "", referencedTable.Schemaname+"."+referencedTable.Relname, referencedTable.Relname) + if c.partitionTablesMap[referencedTableObjName] { + //If reference table is Partitioned table + c.isForeignKeyReferencesPartitionTableDetected = true + } + } + + return nil +} + +func (c *ConstraintIssuesDetector) GetIssues() []QueryIssue { + issues := make([]QueryIssue, 0) + if c.isForeignKeyReferencesPartitionTableDetected { + issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue(DML_QUERY_OBJECT_TYPE, "", c.query)) + } + return issues +} diff --git a/yb-voyager/src/query/queryissue/detectors_test.go b/yb-voyager/src/query/queryissue/detectors_test.go index 16c136d3b..ec70fcc98 100644 --- a/yb-voyager/src/query/queryissue/detectors_test.go +++ b/yb-voyager/src/query/queryissue/detectors_test.go @@ -710,3 +710,13 @@ WHERE JSON_EXISTS(details, '$.price ? (@ > $price)' PASSING 30 AS price);` assert.Equal(t, JSON_QUERY_FUNCTION, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) } + +func TestConstraintIssuesDetector(t *testing.T) { + sql := `CREATE TABLE abc_fk(id int PRIMARY KEY, abc_id INT REFERENCES public.abc(id), val text) ;` + + issues := getDetectorIssues(t, NewConstraintIssuesDetector(sql, map[string]bool{ + "public.abc": true, + }), sql) + assert.Equal(t, 1, len(issues), "Expected 1 issue for SQL: %s", sql) + assert.Equal(t, FOREIGN_KEY_REFERENCED_PARTITIONED_TABLE, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) +} diff --git a/yb-voyager/src/query/queryissue/issues_ddl.go b/yb-voyager/src/query/queryissue/issues_ddl.go index ba1d4cdae..dde4af96b 100644 --- a/yb-voyager/src/query/queryissue/issues_ddl.go +++ b/yb-voyager/src/query/queryissue/issues_ddl.go @@ -465,3 +465,15 @@ var securityInvokerViewIssue = issue.Issue{ func NewSecurityInvokerViewIssue(objectType string, objectName string, SqlStatement string) QueryIssue { return newQueryIssue(securityInvokerViewIssue, objectType, objectName, SqlStatement, map[string]interface{}{}) } + +var foreignKeyReferencesPartitionedTableIssue = issue.Issue{ + Type: FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, + TypeName: FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME, + Suggestion: "Not workaround available ", + GH: "", // TODO + DocsLink: "", // TODO +} + +func NewForeignKeyReferencesPartitionedTableIssue(objectType string, objectName string, SqlStatement string) QueryIssue { + return newQueryIssue(foreignKeyReferencesPartitionedTableIssue, objectType, objectName, SqlStatement, map[string]interface{}{}) +} diff --git a/yb-voyager/src/query/queryissue/issues_ddl_test.go b/yb-voyager/src/query/queryissue/issues_ddl_test.go index d53896781..3878af818 100644 --- a/yb-voyager/src/query/queryissue/issues_ddl_test.go +++ b/yb-voyager/src/query/queryissue/issues_ddl_test.go @@ -27,6 +27,7 @@ import ( "github.com/jackc/pgx/v5" "github.com/stretchr/testify/assert" "github.com/testcontainers/testcontainers-go/modules/yugabytedb" + "github.com/yugabyte/yb-voyager/yb-voyager/src/issue" "github.com/yugabyte/yb-voyager/yb-voyager/src/ybversion" testutils "github.com/yugabyte/yb-voyager/yb-voyager/test/utils" @@ -273,6 +274,19 @@ func testSecurityInvokerView(t *testing.T) { assertErrorCorrectlyThrownForIssueForYBVersion(t, err, "unrecognized parameter", securityInvokerViewIssue) } +func testForeignKeyReferencesPartitionedTableIssue(t *testing.T) { + ctx := context.Background() + conn, err := getConn() + assert.NoError(t, err) + + defer conn.Close(context.Background()) + _, err = conn.Exec(ctx, ` + CREATE TABLE abc1(id int PRIMARY KEY, val text) PARTITION BY RANGE (id); + CREATE TABLE abc_fk(id int PRIMARY KEY, abc_id INT REFERENCES abc1(id), val text) ;`) + + assertErrorCorrectlyThrownForIssueForYBVersion(t, err, `cannot reference partitioned table "abc1"`, foreignKeyReferencesPartitionedTableIssue) +} + func TestDDLIssuesInYBVersion(t *testing.T) { var err error ybVersion := os.Getenv("YB_VERSION") @@ -328,6 +342,10 @@ func TestDDLIssuesInYBVersion(t *testing.T) { success = t.Run(fmt.Sprintf("%s-%s", "multi range datatype", ybVersion), testMultiRangeDatatypeIssue) assert.True(t, success) + success = t.Run(fmt.Sprintf("%s-%s", "security invoker view", ybVersion), testSecurityInvokerView) assert.True(t, success) + + success = t.Run(fmt.Sprintf("%s-%s", "foreign key referenced partitioned table", ybVersion), testForeignKeyReferencesPartitionedTableIssue) + assert.True(t, success) } diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector.go b/yb-voyager/src/query/queryissue/parser_issue_detector.go index c7faf1209..33f4f34b7 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector.go @@ -379,6 +379,7 @@ func (p *ParserIssueDetector) genericIssues(query string) ([]QueryIssue, error) NewCopyCommandUnsupportedConstructsDetector(query), NewJsonConstructorFuncDetector(query), NewJsonQueryFunctionDetector(query), + NewConstraintIssuesDetector(query, p.partitionTablesMap), } 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 e5b6315b0..865a6833b 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -25,9 +25,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/samber/lo" "github.com/stretchr/testify/assert" - testutils "github.com/yugabyte/yb-voyager/yb-voyager/test/utils" "github.com/yugabyte/yb-voyager/yb-voyager/src/ybversion" + testutils "github.com/yugabyte/yb-voyager/yb-voyager/test/utils" ) const ( @@ -754,3 +754,56 @@ func TestCopyUnsupportedConstructIssuesDetected(t *testing.T) { } } } + +func TestForeignKeyReferencesPartitionedTableIssues(t *testing.T) { + requiredDDLs := []string{ + `CREATE TABLE abc1(id int PRIMARY KEY, val text) PARTITION BY RANGE (id);`, + `CREATE TABLE schema1.abc(id int PRIMARY KEY, val text) PARTITION BY RANGE (id);`, + } + stmt1 := `CREATE TABLE abc_fk(id int PRIMARY KEY, abc_id INT REFERENCES abc1(id), val text) ;` + stmt2 := `ALTER TABLE schema1.abc_fk1 +ADD CONSTRAINT fk FOREIGN KEY (abc1_id) +REFERENCES schema1.abc (id); +` + stmt3 := `CREATE TABLE abc_fk ( + id INT PRIMARY KEY, + abc_id INT, + val TEXT, + CONSTRAINT fk_abc FOREIGN KEY (abc_id) REFERENCES abc1(id) +); +` + + stmt4 := `CREATE TABLE schema1.abc_fk(id int PRIMARY KEY, abc_id INT, val text, FOREIGN KEY (abc_id) REFERENCES schema1.abc(id));` + + ddlStmtsWithIssues := map[string][]QueryIssue{ + stmt1: []QueryIssue{ + NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt1), + }, + stmt2: []QueryIssue{ + NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk1", stmt2), + }, + stmt3: []QueryIssue{ + NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt3), + }, + stmt4: []QueryIssue{ + NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk", stmt4), + }, + } + parserIssueDetector := NewParserIssueDetector() + for _, stmt := range requiredDDLs { + err := parserIssueDetector.ParseRequiredDDLs(stmt) + assert.NoError(t, err, "Error parsing required ddl: %s", stmt) + } + for stmt, expectedIssues := range ddlStmtsWithIssues { + 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) + } + } +} diff --git a/yb-voyager/src/query/queryparser/helpers_protomsg.go b/yb-voyager/src/query/queryparser/helpers_protomsg.go index de3204f3c..562aceac4 100644 --- a/yb-voyager/src/query/queryparser/helpers_protomsg.go +++ b/yb-voyager/src/query/queryparser/helpers_protomsg.go @@ -420,6 +420,18 @@ func ProtoAsSelectStmt(msg protoreflect.Message) (*pg_query.SelectStmt, error) { return selectStmtNode, nil } +func GetConstraintNode(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") + } + constNode, ok := protoMsg.(*pg_query.Constraint) + if !ok { + return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_CONSTRAINT_NODE) + } + return constNode, nil +} + /* Example: options:{def_elem:{defname:"security_invoker" arg:{string:{sval:"true"}} defaction:DEFELEM_UNSPEC location:32}} diff --git a/yb-voyager/src/query/queryparser/traversal_proto.go b/yb-voyager/src/query/queryparser/traversal_proto.go index 9ca3ae3ad..26dce005f 100644 --- a/yb-voyager/src/query/queryparser/traversal_proto.go +++ b/yb-voyager/src/query/queryparser/traversal_proto.go @@ -51,6 +51,7 @@ const ( PG_QUERY_JSON_TABLE_NODE = "pg_query.JsonTable" PG_QUERY_VIEWSTMT_NODE = "pg_query.ViewStmt" PG_QUERY_COPYSTSMT_NODE = "pg_query.CopyStmt" + PG_QUERY_CONSTRAINT_NODE = "pg_query.Constraint" ) // function type for processing nodes during traversal From 3c03d6c5ca3e43ae1d91d3a919dab6a682f5eba4 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Thu, 2 Jan 2025 18:39:50 +0530 Subject: [PATCH 2/9] added end-to-end tests in analyze and assessment --- .../dummy-export-dir/schema/tables/table.sql | 2 +- migtests/tests/analyze-schema/expected_issues.json | 14 ++++++++++++-- .../expectedAssessmentReport.json | 14 ++++++++++++-- .../pg_assessment_report.sql | 4 +++- 4 files changed, 28 insertions(+), 6 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 4da2e5f44..a855796a4 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 @@ -27,7 +27,7 @@ CREATE TABLE sales ( -- cases for multi column list partition, to be reported during analyze-schema CREATE TABLE test_1 ( - id numeric NOT NULL, + id numeric NOT NULL REFERENCES sales_data(sales_id), country_code varchar(3), record_type varchar(5), descriptions varchar(50), diff --git a/migtests/tests/analyze-schema/expected_issues.json b/migtests/tests/analyze-schema/expected_issues.json index 0bfd66b0e..2bb0d4f04 100644 --- a/migtests/tests/analyze-schema/expected_issues.json +++ b/migtests/tests/analyze-schema/expected_issues.json @@ -40,6 +40,16 @@ "GH": "", "MinimumVersionsFixedIn": null }, + { + "IssueType": "unsupported_features", + "ObjectType": "TABLE", + "ObjectName": "test_1", + "Reason": "Foreign key constraint references partitioned table", + "SqlStatement": "CREATE TABLE test_1 (\n\tid numeric NOT NULL REFERENCES sales_data(sales_id),\n\tcountry_code varchar(3),\n\trecord_type varchar(5),\n\tdescriptions varchar(50),\n\tPRIMARY KEY (id)\n) PARTITION BY LIST (country_code, record_type) ;", + "Suggestion": "Not workaround available ", + "GH": "", + "MinimumVersionsFixedIn": null + }, { "IssueType": "unsupported_features", "ObjectType": "MVIEW", @@ -416,7 +426,7 @@ "ObjectType": "TABLE", "ObjectName": "test_1", "Reason": "cannot use \"list\" partition strategy with more than one column", - "SqlStatement": "CREATE TABLE test_1 (\n\tid numeric NOT NULL,\n\tcountry_code varchar(3),\n\trecord_type varchar(5),\n\tdescriptions varchar(50),\n\tPRIMARY KEY (id)\n) PARTITION BY LIST (country_code, record_type) ;", + "SqlStatement": "CREATE TABLE test_1 (\n\tid numeric NOT NULL REFERENCES sales_data(sales_id),\n\tcountry_code varchar(3),\n\trecord_type varchar(5),\n\tdescriptions varchar(50),\n\tPRIMARY KEY (id)\n) PARTITION BY LIST (country_code, record_type) ;", "DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/mysql/#multi-column-partition-by-list-is-not-supported", "Suggestion": "Make it a single column partition by list or choose other supported Partitioning methods", "GH": "https://github.com/yugabyte/yb-voyager/issues/699", @@ -1465,7 +1475,7 @@ "ObjectType": "TABLE", "ObjectName": "test_1", "Reason": "insufficient columns in the PRIMARY KEY constraint definition in CREATE TABLE - (country_code, record_type)", - "SqlStatement": "CREATE TABLE test_1 (\n\tid numeric NOT NULL,\n\tcountry_code varchar(3),\n\trecord_type varchar(5),\n\tdescriptions varchar(50),\n\tPRIMARY KEY (id)\n) PARTITION BY LIST (country_code, record_type) ;", + "SqlStatement": "CREATE TABLE test_1 (\n\tid numeric NOT NULL REFERENCES sales_data(sales_id),\n\tcountry_code varchar(3),\n\trecord_type varchar(5),\n\tdescriptions varchar(50),\n\tPRIMARY KEY (id)\n) PARTITION BY LIST (country_code, record_type) ;", "Suggestion": "Add all Partition columns to Primary Key", "GH": "https://github.com/yugabyte/yb-voyager/issues/578", "DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/oracle/#partition-key-column-not-part-of-primary-key-columns", diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index 76e5ffb54..1c1eec2f1 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -45,7 +45,7 @@ { "ObjectType": "TABLE", "TotalCount": 82, - "InvalidCount": 35, + "InvalidCount": 36, "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" }, { @@ -634,6 +634,16 @@ ], "MinimumVersionsFixedIn": null }, + { + "FeatureName": "Foreign key constraint references partitioned table", + "Objects": [ + { + "ObjectName": "public.test_jsonb", + "SqlStatement": "ALTER TABLE ONLY public.test_jsonb\n ADD CONSTRAINT test_jsonb_id_region_fkey FOREIGN KEY (id, region) REFERENCES public.sales_region(id, region);" + } + ], + "MinimumVersionsFixedIn": null + }, { "FeatureName": "Regex Functions", "Objects": [ @@ -1011,7 +1021,7 @@ "SchemaName": "public", "ObjectName": "test_jsonb", "RowCount": 0, - "ColumnCount": 3, + "ColumnCount": 4, "Reads": 0, "Writes": 0, "ReadsPerSecond": 0, 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 7e3c798ea..72e5ea1e7 100644 --- a/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql +++ b/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql @@ -144,7 +144,9 @@ WITH CHECK OPTION; CREATE TABLE public.test_jsonb ( id integer, data jsonb, - data2 text + data2 text, + region text, + FOREIGN KEY (id, region) REFERENCES sales_region(id, region) ); CREATE TABLE public.inet_type ( From ef67431c98a3dc54e52c98ea783e72cc54d38a99 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Thu, 2 Jan 2025 19:21:28 +0530 Subject: [PATCH 3/9] fix undefineds --- yb-voyager/src/query/queryissue/detectors_test.go | 2 +- .../src/query/queryissue/parser_issue_detector_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/yb-voyager/src/query/queryissue/detectors_test.go b/yb-voyager/src/query/queryissue/detectors_test.go index ec70fcc98..fbda2a02d 100644 --- a/yb-voyager/src/query/queryissue/detectors_test.go +++ b/yb-voyager/src/query/queryissue/detectors_test.go @@ -718,5 +718,5 @@ func TestConstraintIssuesDetector(t *testing.T) { "public.abc": true, }), sql) assert.Equal(t, 1, len(issues), "Expected 1 issue for SQL: %s", sql) - assert.Equal(t, FOREIGN_KEY_REFERENCED_PARTITIONED_TABLE, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) + assert.Equal(t, FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) } 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 865a6833b..8043ff30b 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -777,16 +777,16 @@ REFERENCES schema1.abc (id); ddlStmtsWithIssues := map[string][]QueryIssue{ stmt1: []QueryIssue{ - NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt1), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt1), }, stmt2: []QueryIssue{ - NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk1", stmt2), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk1", stmt2), }, stmt3: []QueryIssue{ - NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt3), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt3), }, stmt4: []QueryIssue{ - NewForeignKeyReferencedPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk", stmt4), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk", stmt4), }, } parserIssueDetector := NewParserIssueDetector() From 4c8267da7256347fd695ba101f93e991cea57a39 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Thu, 2 Jan 2025 20:01:20 +0530 Subject: [PATCH 4/9] fiz suggestion field --- migtests/tests/analyze-schema/expected_issues.json | 2 +- yb-voyager/src/query/queryissue/issues_ddl.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/migtests/tests/analyze-schema/expected_issues.json b/migtests/tests/analyze-schema/expected_issues.json index 2bb0d4f04..84c636bf8 100644 --- a/migtests/tests/analyze-schema/expected_issues.json +++ b/migtests/tests/analyze-schema/expected_issues.json @@ -46,7 +46,7 @@ "ObjectName": "test_1", "Reason": "Foreign key constraint references partitioned table", "SqlStatement": "CREATE TABLE test_1 (\n\tid numeric NOT NULL REFERENCES sales_data(sales_id),\n\tcountry_code varchar(3),\n\trecord_type varchar(5),\n\tdescriptions varchar(50),\n\tPRIMARY KEY (id)\n) PARTITION BY LIST (country_code, record_type) ;", - "Suggestion": "Not workaround available ", + "Suggestion": "No workaround available ", "GH": "", "MinimumVersionsFixedIn": null }, diff --git a/yb-voyager/src/query/queryissue/issues_ddl.go b/yb-voyager/src/query/queryissue/issues_ddl.go index dde4af96b..8979c7aba 100644 --- a/yb-voyager/src/query/queryissue/issues_ddl.go +++ b/yb-voyager/src/query/queryissue/issues_ddl.go @@ -469,7 +469,7 @@ func NewSecurityInvokerViewIssue(objectType string, objectName string, SqlStatem var foreignKeyReferencesPartitionedTableIssue = issue.Issue{ Type: FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, TypeName: FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME, - Suggestion: "Not workaround available ", + Suggestion: "No workaround available ", GH: "", // TODO DocsLink: "", // TODO } From 7050196571453f02ec55be2a754b3b45758f962f Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Thu, 2 Jan 2025 20:52:12 +0530 Subject: [PATCH 5/9] changed approach to use the DDL processor for this case as need the constraint name as well --- .../tests/analyze-schema/expected_issues.json | 2 +- .../expectedAssessmentReport.json | 2 +- yb-voyager/cmd/analyzeSchema.go | 1 + yb-voyager/src/query/queryissue/detectors.go | 89 +++++++++---------- .../src/query/queryissue/detectors_ddl.go | 20 +++++ .../src/query/queryissue/detectors_test.go | 18 ++-- yb-voyager/src/query/queryissue/issues_ddl.go | 7 +- .../query/queryissue/parser_issue_detector.go | 2 +- .../queryissue/parser_issue_detector_test.go | 9 +- .../src/query/queryparser/ddl_processor.go | 81 ++++++++++------- 10 files changed, 138 insertions(+), 93 deletions(-) diff --git a/migtests/tests/analyze-schema/expected_issues.json b/migtests/tests/analyze-schema/expected_issues.json index 84c636bf8..ca9d23c02 100644 --- a/migtests/tests/analyze-schema/expected_issues.json +++ b/migtests/tests/analyze-schema/expected_issues.json @@ -43,7 +43,7 @@ { "IssueType": "unsupported_features", "ObjectType": "TABLE", - "ObjectName": "test_1", + "ObjectName": "test_1, constraint: (test_1_id_fkey)", "Reason": "Foreign key constraint references partitioned table", "SqlStatement": "CREATE TABLE test_1 (\n\tid numeric NOT NULL REFERENCES sales_data(sales_id),\n\tcountry_code varchar(3),\n\trecord_type varchar(5),\n\tdescriptions varchar(50),\n\tPRIMARY KEY (id)\n) PARTITION BY LIST (country_code, record_type) ;", "Suggestion": "No workaround available ", diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index 1c1eec2f1..9332320b1 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -638,7 +638,7 @@ "FeatureName": "Foreign key constraint references partitioned table", "Objects": [ { - "ObjectName": "public.test_jsonb", + "ObjectName": "public.test_jsonb, constraint: (test_jsonb_id_region_fkey)", "SqlStatement": "ALTER TABLE ONLY public.test_jsonb\n ADD CONSTRAINT test_jsonb_id_region_fkey FOREIGN KEY (id, region) REFERENCES public.sales_region(id, region);" } ], diff --git a/yb-voyager/cmd/analyzeSchema.go b/yb-voyager/cmd/analyzeSchema.go index a4dda2b03..a6ec455e3 100644 --- a/yb-voyager/cmd/analyzeSchema.go +++ b/yb-voyager/cmd/analyzeSchema.go @@ -654,6 +654,7 @@ func convertIssueInstanceToAnalyzeIssue(issueInstance queryissue.QueryIssue, fil queryissue.EXCLUSION_CONSTRAINTS, queryissue.DEFERRABLE_CONSTRAINTS, queryissue.PK_UK_ON_COMPLEX_DATATYPE, + queryissue.FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, } /* TODO: diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index 6e2fb60bb..bdad95d9f 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -19,7 +19,6 @@ import ( "slices" mapset "github.com/deckarep/golang-set/v2" - "github.com/samber/lo" log "github.com/sirupsen/logrus" "google.golang.org/protobuf/reflect/protoreflect" @@ -392,47 +391,47 @@ func (d *JsonQueryFunctionDetector) GetIssues() []QueryIssue { return issues } -type ConstraintIssuesDetector struct { - query string - partitionTablesMap map[string]bool - isForeignKeyReferencesPartitionTableDetected bool -} - -func NewConstraintIssuesDetector(query string, partitionTablesMap map[string]bool) *ConstraintIssuesDetector { - return &ConstraintIssuesDetector{ - query: query, - partitionTablesMap: partitionTablesMap, - } -} - -func (c *ConstraintIssuesDetector) Detect(msg protoreflect.Message) error { - if queryparser.GetMsgFullName(msg) != queryparser.PG_QUERY_CONSTRAINT_NODE { - return nil - } - - constraintNode, err := queryparser.GetConstraintNode(msg) - if err != nil { - return err - } - - switch constraintNode.Contype { - case queryparser.FOREIGN_CONSTR_TYPE: - referencedTable := constraintNode.Pktable - //constraints:{constraint:{contype:CONSTR_FOREIGN initially_valid:true pktable:{relname:"abc1" inh:true relpersistence:"p" location:121} - referencedTableObjName := lo.Ternary(referencedTable.Schemaname != "", referencedTable.Schemaname+"."+referencedTable.Relname, referencedTable.Relname) - if c.partitionTablesMap[referencedTableObjName] { - //If reference table is Partitioned table - c.isForeignKeyReferencesPartitionTableDetected = true - } - } - - return nil -} - -func (c *ConstraintIssuesDetector) GetIssues() []QueryIssue { - issues := make([]QueryIssue, 0) - if c.isForeignKeyReferencesPartitionTableDetected { - issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue(DML_QUERY_OBJECT_TYPE, "", c.query)) - } - return issues -} +// type ConstraintIssuesDetector struct { +// query string +// partitionTablesMap map[string]bool +// isForeignKeyReferencesPartitionTableDetected bool +// } + +// func NewConstraintIssuesDetector(query string, partitionTablesMap map[string]bool) *ConstraintIssuesDetector { +// return &ConstraintIssuesDetector{ +// query: query, +// partitionTablesMap: partitionTablesMap, +// } +// } + +// func (c *ConstraintIssuesDetector) Detect(msg protoreflect.Message) error { +// if queryparser.GetMsgFullName(msg) != queryparser.PG_QUERY_CONSTRAINT_NODE { +// return nil +// } + +// constraintNode, err := queryparser.GetConstraintNode(msg) +// if err != nil { +// return err +// } + +// switch constraintNode.Contype { +// case queryparser.FOREIGN_CONSTR_TYPE: +// referencedTable := constraintNode.Pktable +// //constraints:{constraint:{contype:CONSTR_FOREIGN initially_valid:true pktable:{relname:"abc1" inh:true relpersistence:"p" location:121} +// referencedTableObjName := lo.Ternary(referencedTable.Schemaname != "", referencedTable.Schemaname+"."+referencedTable.Relname, referencedTable.Relname) +// if c.partitionTablesMap[referencedTableObjName] { +// //If reference table is Partitioned table +// c.isForeignKeyReferencesPartitionTableDetected = true +// } +// } + +// return nil +// } + +// func (c *ConstraintIssuesDetector) GetIssues() []QueryIssue { +// issues := make([]QueryIssue, 0) +// if c.isForeignKeyReferencesPartitionTableDetected { +// // issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue(DML_QUERY_OBJECT_TYPE, "", c.query)) +// } +// return issues +// } diff --git a/yb-voyager/src/query/queryissue/detectors_ddl.go b/yb-voyager/src/query/queryissue/detectors_ddl.go index 595680f9a..35a712f99 100644 --- a/yb-voyager/src/query/queryissue/detectors_ddl.go +++ b/yb-voyager/src/query/queryissue/detectors_ddl.go @@ -95,6 +95,15 @@ func (d *TableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIss )) } + if c.ConstraintType == queryparser.FOREIGN_CONSTR_TYPE && d.partitionTablesMap[c.ReferencedTable] { + issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue( + TABLE_OBJECT_TYPE, + table.GetObjectName(), + "", + c.ConstraintName, + )) + } + if c.IsPrimaryKeyORUniqueConstraint() { for _, col := range c.Columns { unsupportedColumnsForTable, ok := d.columnsWithUnsupportedIndexDatatypes[table.GetObjectName()] @@ -444,6 +453,17 @@ func (aid *AlterTableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]Q )) } + if alter.ConstraintType == queryparser.FOREIGN_CONSTR_TYPE && + aid.partitionTablesMap[alter.ConstraintReferencedTable] { + //FK constraint references partitioned table + issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue( + TABLE_OBJECT_TYPE, + alter.GetObjectName(), + "", + alter.ConstraintName, + )) + } + if alter.ConstraintType == queryparser.PRIMARY_CONSTR_TYPE && aid.partitionTablesMap[alter.GetObjectName()] { issues = append(issues, NewAlterTableAddPKOnPartiionIssue( diff --git a/yb-voyager/src/query/queryissue/detectors_test.go b/yb-voyager/src/query/queryissue/detectors_test.go index fbda2a02d..d6c706ac3 100644 --- a/yb-voyager/src/query/queryissue/detectors_test.go +++ b/yb-voyager/src/query/queryissue/detectors_test.go @@ -711,12 +711,12 @@ WHERE JSON_EXISTS(details, '$.price ? (@ > $price)' PASSING 30 AS price);` } -func TestConstraintIssuesDetector(t *testing.T) { - sql := `CREATE TABLE abc_fk(id int PRIMARY KEY, abc_id INT REFERENCES public.abc(id), val text) ;` - - issues := getDetectorIssues(t, NewConstraintIssuesDetector(sql, map[string]bool{ - "public.abc": true, - }), sql) - assert.Equal(t, 1, len(issues), "Expected 1 issue for SQL: %s", sql) - assert.Equal(t, FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) -} +// func TestConstraintIssuesDetector(t *testing.T) { +// sql := `CREATE TABLE abc_fk(id int PRIMARY KEY, abc_id INT REFERENCES public.abc(id), val text) ;` + +// issues := getDetectorIssues(t, NewConstraintIssuesDetector(sql, map[string]bool{ +// "public.abc": true, +// }), sql) +// assert.Equal(t, 1, len(issues), "Expected 1 issue for SQL: %s", sql) +// assert.Equal(t, FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) +// } diff --git a/yb-voyager/src/query/queryissue/issues_ddl.go b/yb-voyager/src/query/queryissue/issues_ddl.go index 8979c7aba..892f1c3d8 100644 --- a/yb-voyager/src/query/queryissue/issues_ddl.go +++ b/yb-voyager/src/query/queryissue/issues_ddl.go @@ -474,6 +474,9 @@ var foreignKeyReferencesPartitionedTableIssue = issue.Issue{ DocsLink: "", // TODO } -func NewForeignKeyReferencesPartitionedTableIssue(objectType string, objectName string, SqlStatement string) QueryIssue { - return newQueryIssue(foreignKeyReferencesPartitionedTableIssue, objectType, objectName, SqlStatement, map[string]interface{}{}) +func NewForeignKeyReferencesPartitionedTableIssue(objectType string, objectName string, SqlStatement string, constraintName string) QueryIssue { + details := map[string]interface{}{ + CONSTRAINT_NAME: constraintName, + } + return newQueryIssue(foreignKeyReferencesPartitionedTableIssue, objectType, objectName, SqlStatement, details) } diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector.go b/yb-voyager/src/query/queryissue/parser_issue_detector.go index 33f4f34b7..53e3d638a 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector.go @@ -379,7 +379,7 @@ func (p *ParserIssueDetector) genericIssues(query string) ([]QueryIssue, error) NewCopyCommandUnsupportedConstructsDetector(query), NewJsonConstructorFuncDetector(query), NewJsonQueryFunctionDetector(query), - NewConstraintIssuesDetector(query, p.partitionTablesMap), + // NewConstraintIssuesDetector(query, p.partitionTablesMap), } 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 8043ff30b..345051395 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -777,16 +777,16 @@ REFERENCES schema1.abc (id); ddlStmtsWithIssues := map[string][]QueryIssue{ stmt1: []QueryIssue{ - NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt1), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt1, "abc_fk_abc_id_fkey"), }, stmt2: []QueryIssue{ - NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk1", stmt2), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk1", stmt2, "fk"), }, stmt3: []QueryIssue{ - NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt3), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt3, "fk_abc"), }, stmt4: []QueryIssue{ - NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk", stmt4), + NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk", stmt4, "abc_fk_abc_id_fkey"), }, } parserIssueDetector := NewParserIssueDetector() @@ -796,6 +796,7 @@ REFERENCES schema1.abc (id); } for stmt, expectedIssues := range ddlStmtsWithIssues { issues, err := parserIssueDetector.GetDDLIssues(stmt, ybversion.LatestStable) + fmt.Printf("%v", issues) 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) diff --git a/yb-voyager/src/query/queryparser/ddl_processor.go b/yb-voyager/src/query/queryparser/ddl_processor.go index a14485cd8..974a35646 100644 --- a/yb-voyager/src/query/queryparser/ddl_processor.go +++ b/yb-voyager/src/query/queryparser/ddl_processor.go @@ -122,7 +122,28 @@ func (tableProcessor *TableProcessor) Process(parseTree *pg_query.ParseResult) ( } // Parse columns and their properties - for _, element := range createTableNode.CreateStmt.TableElts { + tableProcessor.parseTableElts(createTableNode.CreateStmt.TableElts, table) + + if table.IsPartitioned { + + partitionElements := createTableNode.CreateStmt.GetPartspec().GetPartParams() + table.PartitionStrategy = createTableNode.CreateStmt.GetPartspec().GetStrategy() + + for _, partElem := range partitionElements { + if partElem.GetPartitionElem().GetExpr() != nil { + table.IsExpressionPartition = true + } else { + table.PartitionColumns = append(table.PartitionColumns, partElem.GetPartitionElem().GetName()) + } + } + } + + return table, nil +} + +func (tableProcessor *TableProcessor) parseTableElts(tableElts []*pg_query.Node, table *Table) { + + for _, element := range tableElts { if element.GetColumnDef() != nil { if tableProcessor.isGeneratedColumn(element.GetColumnDef()) { table.GeneratedColumns = append(table.GeneratedColumns, element.GetColumnDef().Colname) @@ -172,7 +193,7 @@ func (tableProcessor *TableProcessor) Process(parseTree *pg_query.ParseResult) ( table.Constraints[len(table.Constraints)-1] = lastConstraint } } else { - table.addConstraint(constraint.Contype, []string{colName}, constraint.Conname, false) + table.addConstraint(constraint.Contype, []string{colName}, constraint.Conname, false, constraint.Pktable) } } } @@ -190,33 +211,25 @@ func (tableProcessor *TableProcessor) Process(parseTree *pg_query.ParseResult) ( constraint := element.GetConstraint() conType := element.GetConstraint().Contype columns := parseColumnsFromKeys(constraint.GetKeys()) - if conType == EXCLUSION_CONSTR_TYPE { + switch conType { + case EXCLUSION_CONSTR_TYPE: //In case CREATE DDL has EXCLUDE USING gist(room_id '=', time_range WITH &&) - it will be included in columns but won't have columnDef as its a constraint exclusions := constraint.GetExclusions() //exclusions:{list:{items:{index_elem:{name:"room_id" ordering:SORTBY_DEFAULT nulls_ordering:SORTBY_NULLS_DEFAULT}} //items:{list:{items:{string:{sval:"="}}}}}} columns = tableProcessor.parseColumnsFromExclusions(exclusions) + case FOREIGN_CONSTR_TYPE: + // In case of Foreign key constraint if it is present at the end of table column definition + fkAttrs := constraint.FkAttrs + //CREATE TABLE schema1.abc_fk(id int, abc_id INT, val text, PRIMARY KEY(id), FOREIGN KEY (abc_id) REFERENCES schema1.abc(id)); + //table_elts:{constraint:{contype:CONSTR_FOREIGN initially_valid:true pktable:{schemaname:"schema1" relname:"abc" inh:true relpersistence:"p" location:109} + //fk_attrs:{string:{sval:"abc_id"}} pk_attrs:{string:{sval:"id"}} + columns = parseColumnsFromKeys(fkAttrs) } - table.addConstraint(conType, columns, constraint.Conname, constraint.Deferrable) + table.addConstraint(conType, columns, constraint.Conname, constraint.Deferrable, constraint.Pktable) } } - - if table.IsPartitioned { - - partitionElements := createTableNode.CreateStmt.GetPartspec().GetPartParams() - table.PartitionStrategy = createTableNode.CreateStmt.GetPartspec().GetStrategy() - - for _, partElem := range partitionElements { - if partElem.GetPartitionElem().GetExpr() != nil { - table.IsExpressionPartition = true - } else { - table.PartitionColumns = append(table.PartitionColumns, partElem.GetPartitionElem().GetName()) - } - } - } - - return table, nil } func (tableProcessor *TableProcessor) checkInheritance(createTableNode *pg_query.Node_CreateStmt) bool { @@ -290,10 +303,11 @@ func (tc *TableColumn) GetFullTypeName() string { } type TableConstraint struct { - ConstraintType pg_query.ConstrType - ConstraintName string - IsDeferrable bool - Columns []string + ConstraintType pg_query.ConstrType + ConstraintName string + IsDeferrable bool + ReferencedTable string + Columns []string } func (c *TableConstraint) IsPrimaryKeyORUniqueConstraint() bool { @@ -345,7 +359,7 @@ func (t *Table) UniqueKeyColumns() []string { return uniqueCols } -func (t *Table) addConstraint(conType pg_query.ConstrType, columns []string, specifiedConName string, deferrable bool) { +func (t *Table) addConstraint(conType pg_query.ConstrType, columns []string, specifiedConName string, deferrable bool, referencedTable *pg_query.RangeVar) { tc := TableConstraint{ ConstraintType: conType, Columns: columns, @@ -354,6 +368,9 @@ func (t *Table) addConstraint(conType pg_query.ConstrType, columns []string, spe generatedConName := tc.generateConstraintName(t.TableName) conName := lo.Ternary(specifiedConName == "", generatedConName, specifiedConName) tc.ConstraintName = conName + if conType == FOREIGN_CONSTR_TYPE { + tc.ReferencedTable = lo.Ternary(referencedTable.Schemaname != "", referencedTable.Schemaname+"."+referencedTable.Relname, referencedTable.Relname) + } t.Constraints = append(t.Constraints, tc) } @@ -575,6 +592,9 @@ func (atProcessor *AlterTableProcessor) Process(parseTree *pg_query.ParseResult) alter.IsDeferrable = constraint.Deferrable alter.ConstraintNotValid = constraint.SkipValidation // this is set for the NOT VALID clause alter.ConstraintColumns = parseColumnsFromKeys(constraint.GetKeys()) + if alter.ConstraintType == FOREIGN_CONSTR_TYPE { + alter.ConstraintReferencedTable = lo.Ternary(constraint.Pktable.Schemaname != "", constraint.Pktable.Schemaname+"."+constraint.Pktable.Relname, constraint.Pktable.Relname) + } case pg_query.AlterTableType_AT_DisableRule: /* @@ -604,11 +624,12 @@ type AlterTable struct { NumSetAttributes int NumStorageOptions int //In case AlterType - ADD_CONSTRAINT - ConstraintType pg_query.ConstrType - ConstraintName string - ConstraintNotValid bool - IsDeferrable bool - ConstraintColumns []string + ConstraintType pg_query.ConstrType + ConstraintName string + ConstraintNotValid bool + ConstraintReferencedTable string + IsDeferrable bool + ConstraintColumns []string } func (a *AlterTable) GetObjectName() string { From 17c6c0a987b226a9e57ea6cc3ba9ee6d36d6cac9 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Thu, 2 Jan 2025 20:58:19 +0530 Subject: [PATCH 6/9] clean up --- yb-voyager/src/query/queryissue/detectors.go | 45 ------------------- .../src/query/queryissue/detectors_test.go | 10 ----- .../query/queryissue/parser_issue_detector.go | 1 - .../queryissue/parser_issue_detector_test.go | 1 - 4 files changed, 57 deletions(-) diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index bdad95d9f..c834fa6db 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -390,48 +390,3 @@ func (d *JsonQueryFunctionDetector) GetIssues() []QueryIssue { } return issues } - -// type ConstraintIssuesDetector struct { -// query string -// partitionTablesMap map[string]bool -// isForeignKeyReferencesPartitionTableDetected bool -// } - -// func NewConstraintIssuesDetector(query string, partitionTablesMap map[string]bool) *ConstraintIssuesDetector { -// return &ConstraintIssuesDetector{ -// query: query, -// partitionTablesMap: partitionTablesMap, -// } -// } - -// func (c *ConstraintIssuesDetector) Detect(msg protoreflect.Message) error { -// if queryparser.GetMsgFullName(msg) != queryparser.PG_QUERY_CONSTRAINT_NODE { -// return nil -// } - -// constraintNode, err := queryparser.GetConstraintNode(msg) -// if err != nil { -// return err -// } - -// switch constraintNode.Contype { -// case queryparser.FOREIGN_CONSTR_TYPE: -// referencedTable := constraintNode.Pktable -// //constraints:{constraint:{contype:CONSTR_FOREIGN initially_valid:true pktable:{relname:"abc1" inh:true relpersistence:"p" location:121} -// referencedTableObjName := lo.Ternary(referencedTable.Schemaname != "", referencedTable.Schemaname+"."+referencedTable.Relname, referencedTable.Relname) -// if c.partitionTablesMap[referencedTableObjName] { -// //If reference table is Partitioned table -// c.isForeignKeyReferencesPartitionTableDetected = true -// } -// } - -// return nil -// } - -// func (c *ConstraintIssuesDetector) GetIssues() []QueryIssue { -// issues := make([]QueryIssue, 0) -// if c.isForeignKeyReferencesPartitionTableDetected { -// // issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue(DML_QUERY_OBJECT_TYPE, "", c.query)) -// } -// return issues -// } diff --git a/yb-voyager/src/query/queryissue/detectors_test.go b/yb-voyager/src/query/queryissue/detectors_test.go index d6c706ac3..16c136d3b 100644 --- a/yb-voyager/src/query/queryissue/detectors_test.go +++ b/yb-voyager/src/query/queryissue/detectors_test.go @@ -710,13 +710,3 @@ WHERE JSON_EXISTS(details, '$.price ? (@ > $price)' PASSING 30 AS price);` assert.Equal(t, JSON_QUERY_FUNCTION, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) } - -// func TestConstraintIssuesDetector(t *testing.T) { -// sql := `CREATE TABLE abc_fk(id int PRIMARY KEY, abc_id INT REFERENCES public.abc(id), val text) ;` - -// issues := getDetectorIssues(t, NewConstraintIssuesDetector(sql, map[string]bool{ -// "public.abc": true, -// }), sql) -// assert.Equal(t, 1, len(issues), "Expected 1 issue for SQL: %s", sql) -// assert.Equal(t, FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql) -// } diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector.go b/yb-voyager/src/query/queryissue/parser_issue_detector.go index 53e3d638a..c7faf1209 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector.go @@ -379,7 +379,6 @@ func (p *ParserIssueDetector) genericIssues(query string) ([]QueryIssue, error) NewCopyCommandUnsupportedConstructsDetector(query), NewJsonConstructorFuncDetector(query), NewJsonQueryFunctionDetector(query), - // NewConstraintIssuesDetector(query, p.partitionTablesMap), } 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 345051395..8819230e7 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -796,7 +796,6 @@ REFERENCES schema1.abc (id); } for stmt, expectedIssues := range ddlStmtsWithIssues { issues, err := parserIssueDetector.GetDDLIssues(stmt, ybversion.LatestStable) - fmt.Printf("%v", issues) 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) From c69c1021f71876603e27ffd8489e3ca2c883c2d7 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Thu, 2 Jan 2025 21:11:09 +0530 Subject: [PATCH 7/9] comments --- yb-voyager/src/query/queryparser/ddl_processor.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/yb-voyager/src/query/queryparser/ddl_processor.go b/yb-voyager/src/query/queryparser/ddl_processor.go index 974a35646..3de0248c6 100644 --- a/yb-voyager/src/query/queryparser/ddl_processor.go +++ b/yb-voyager/src/query/queryparser/ddl_processor.go @@ -193,6 +193,13 @@ func (tableProcessor *TableProcessor) parseTableElts(tableElts []*pg_query.Node, table.Constraints[len(table.Constraints)-1] = lastConstraint } } else { + /* + table_elts:{column_def:{colname:"abc_id" type_name:{names:{string:{sval:"pg_catalog"}} names:{string:{sval:"int4"}} typemod:-1 + location:45} is_local:true constraints:{constraint:{contype:CONSTR_FOREIGN initially_valid:true pktable:{schemaname:"schema1" + relname:"abc" inh:true relpersistence:"p" location:60} pk_attrs:{string:{sval:"id"}} fk_matchtype:"s" fk_upd_action:"a" fk_del_action:"a" + + In case of FKs there is field called PkTable which has reference table information + */ table.addConstraint(constraint.Contype, []string{colName}, constraint.Conname, false, constraint.Pktable) } } @@ -593,6 +600,12 @@ func (atProcessor *AlterTableProcessor) Process(parseTree *pg_query.ParseResult) alter.ConstraintNotValid = constraint.SkipValidation // this is set for the NOT VALID clause alter.ConstraintColumns = parseColumnsFromKeys(constraint.GetKeys()) if alter.ConstraintType == FOREIGN_CONSTR_TYPE { + /* + alter_table_cmd:{subtype:AT_AddConstraint def:{constraint:{contype:CONSTR_FOREIGN conname:"fk" initially_valid:true + pktable:{schemaname:"schema1" relname:"abc" inh:true relpersistence:"p" + In case of FKs the reference table is in PKTable field and columns are in FkAttrs + */ + alter.ConstraintColumns = parseColumnsFromKeys(constraint.FkAttrs) alter.ConstraintReferencedTable = lo.Ternary(constraint.Pktable.Schemaname != "", constraint.Pktable.Schemaname+"."+constraint.Pktable.Relname, constraint.Pktable.Relname) } From c1788840a9c2bde76bb7f5e3047a2ad03d4be680 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Mon, 6 Jan 2025 14:38:00 +0530 Subject: [PATCH 8/9] review comments --- yb-voyager/cmd/exportSchema.go | 3 +- .../src/query/queryparser/ddl_processor.go | 36 ++++++++++--------- .../src/query/queryparser/helpers_protomsg.go | 12 ------- .../src/query/queryparser/helpers_struct.go | 9 ++--- .../src/query/queryparser/object_collector.go | 10 +++--- yb-voyager/src/utils/utils.go | 4 +++ 6 files changed, 36 insertions(+), 38 deletions(-) diff --git a/yb-voyager/cmd/exportSchema.go b/yb-voyager/cmd/exportSchema.go index 22de944ab..c202a5523 100644 --- a/yb-voyager/cmd/exportSchema.go +++ b/yb-voyager/cmd/exportSchema.go @@ -467,8 +467,7 @@ func applyShardingRecommendationIfMatching(sqlInfo *sqlInfo, shardedTables []str } // true -> oracle, false -> PG - parsedObjectName := lo.Ternary(relation.Schemaname == "", relation.Relname, - relation.Schemaname+"."+relation.Relname) + parsedObjectName := utils.GetObjectName(relation.Schemaname, relation.Relname) match := false switch source.DBType { diff --git a/yb-voyager/src/query/queryparser/ddl_processor.go b/yb-voyager/src/query/queryparser/ddl_processor.go index 3de0248c6..469353f10 100644 --- a/yb-voyager/src/query/queryparser/ddl_processor.go +++ b/yb-voyager/src/query/queryparser/ddl_processor.go @@ -23,6 +23,8 @@ import ( pg_query "github.com/pganalyze/pg_query_go/v6" "github.com/samber/lo" log "github.com/sirupsen/logrus" + + "github.com/yugabyte/yb-voyager/yb-voyager/src/utils" ) // Base parser interface @@ -142,7 +144,11 @@ func (tableProcessor *TableProcessor) Process(parseTree *pg_query.ParseResult) ( } func (tableProcessor *TableProcessor) parseTableElts(tableElts []*pg_query.Node, table *Table) { + /* + Parsing the table elements liek column definitions constraint basically all the things inside the () of CREATE TABLE test(id int, CONSTRAINT Pk PRIMARY KEY (id)....); + storing all the information of columns - name, typename, isArraytype and constraints - constraint name, columns involved, type of constraint, is deferrable or not + */ for _, element := range tableElts { if element.GetColumnDef() != nil { if tableProcessor.isGeneratedColumn(element.GetColumnDef()) { @@ -306,7 +312,7 @@ type TableColumn struct { } func (tc *TableColumn) GetFullTypeName() string { - return lo.Ternary(tc.TypeSchema != "", tc.TypeSchema+"."+tc.TypeName, tc.TypeName) + return utils.GetObjectName(tc.TypeSchema, tc.TypeName) } type TableConstraint struct { @@ -340,8 +346,7 @@ func (c *TableConstraint) generateConstraintName(tableName string) string { } func (t *Table) GetObjectName() string { - qualifiedTable := lo.Ternary(t.SchemaName != "", fmt.Sprintf("%s.%s", t.SchemaName, t.TableName), t.TableName) - return qualifiedTable + return utils.GetObjectName(t.SchemaName, t.TableName) } func (t *Table) GetSchemaName() string { return t.SchemaName } @@ -376,7 +381,7 @@ func (t *Table) addConstraint(conType pg_query.ConstrType, columns []string, spe conName := lo.Ternary(specifiedConName == "", generatedConName, specifiedConName) tc.ConstraintName = conName if conType == FOREIGN_CONSTR_TYPE { - tc.ReferencedTable = lo.Ternary(referencedTable.Schemaname != "", referencedTable.Schemaname+"."+referencedTable.Relname, referencedTable.Relname) + tc.ReferencedTable = utils.GetObjectName(referencedTable.Schemaname, referencedTable.Relname) } t.Constraints = append(t.Constraints, tc) } @@ -428,7 +433,7 @@ type ForeignTable struct { } func (f *ForeignTable) GetObjectName() string { - return lo.Ternary(f.SchemaName != "", f.SchemaName+"."+f.TableName, f.TableName) + return utils.GetObjectName(f.SchemaName, f.TableName) } func (f *ForeignTable) GetSchemaName() string { return f.SchemaName } @@ -519,7 +524,7 @@ type IndexParam struct { } func (indexParam *IndexParam) GetFullExprCastTypeName() string { - return lo.Ternary(indexParam.ExprCastTypeSchema != "", indexParam.ExprCastTypeSchema+"."+indexParam.ExprCastTypeName, indexParam.ExprCastTypeName) + return utils.GetObjectName(indexParam.ExprCastTypeSchema, indexParam.ExprCastTypeName) } func (i *Index) GetObjectName() string { @@ -528,7 +533,7 @@ func (i *Index) GetObjectName() string { func (i *Index) GetSchemaName() string { return i.SchemaName } func (i *Index) GetTableName() string { - return lo.Ternary(i.SchemaName != "", fmt.Sprintf("%s.%s", i.SchemaName, i.TableName), i.TableName) + return utils.GetObjectName(i.SchemaName, i.TableName) } func (i *Index) GetObjectType() string { return INDEX_OBJECT_TYPE } @@ -606,7 +611,7 @@ func (atProcessor *AlterTableProcessor) Process(parseTree *pg_query.ParseResult) In case of FKs the reference table is in PKTable field and columns are in FkAttrs */ alter.ConstraintColumns = parseColumnsFromKeys(constraint.FkAttrs) - alter.ConstraintReferencedTable = lo.Ternary(constraint.Pktable.Schemaname != "", constraint.Pktable.Schemaname+"."+constraint.Pktable.Relname, constraint.Pktable.Relname) + alter.ConstraintReferencedTable = utils.GetObjectName(constraint.Pktable.Schemaname, constraint.Pktable.Relname) } case pg_query.AlterTableType_AT_DisableRule: @@ -646,8 +651,7 @@ type AlterTable struct { } func (a *AlterTable) GetObjectName() string { - qualifiedTable := lo.Ternary(a.SchemaName != "", fmt.Sprintf("%s.%s", a.SchemaName, a.TableName), a.TableName) - return qualifiedTable + return utils.GetObjectName(a.SchemaName, a.TableName) } func (a *AlterTable) GetSchemaName() string { return a.SchemaName } @@ -712,7 +716,7 @@ type Policy struct { } func (p *Policy) GetObjectName() string { - qualifiedTable := lo.Ternary(p.SchemaName != "", fmt.Sprintf("%s.%s", p.SchemaName, p.TableName), p.TableName) + qualifiedTable := utils.GetObjectName(p.SchemaName, p.TableName) return fmt.Sprintf("%s ON %s", p.PolicyName, qualifiedTable) } func (p *Policy) GetSchemaName() string { return p.SchemaName } @@ -788,7 +792,7 @@ func (t *Trigger) GetObjectName() string { } func (t *Trigger) GetTableName() string { - return lo.Ternary(t.SchemaName != "", fmt.Sprintf("%s.%s", t.SchemaName, t.TableName), t.TableName) + return utils.GetObjectName(t.SchemaName, t.TableName) } func (t *Trigger) GetSchemaName() string { return t.SchemaName } @@ -865,7 +869,7 @@ type CreateType struct { } func (c *CreateType) GetObjectName() string { - return lo.Ternary(c.SchemaName != "", fmt.Sprintf("%s.%s", c.SchemaName, c.TypeName), c.TypeName) + return utils.GetObjectName(c.SchemaName, c.TypeName) } func (c *CreateType) GetSchemaName() string { return c.SchemaName } @@ -887,7 +891,7 @@ func (v *ViewProcessor) Process(parseTree *pg_query.ParseResult) (DDLObject, err viewSchemaName := viewNode.ViewStmt.View.Schemaname viewName := viewNode.ViewStmt.View.Relname - qualifiedViewName := lo.Ternary(viewSchemaName == "", viewName, viewSchemaName+"."+viewName) + qualifiedViewName := utils.GetObjectName(viewSchemaName, viewName) /* view_stmt:{view:{schemaname:"public" relname:"invoker_view" inh:true relpersistence:"p" location:12} @@ -920,7 +924,7 @@ type View struct { } func (v *View) GetObjectName() string { - return lo.Ternary(v.SchemaName != "", fmt.Sprintf("%s.%s", v.SchemaName, v.ViewName), v.ViewName) + return utils.GetObjectName(v.SchemaName, v.ViewName) } func (v *View) GetSchemaName() string { return v.SchemaName } @@ -952,7 +956,7 @@ type MView struct { } func (mv *MView) GetObjectName() string { - return lo.Ternary(mv.SchemaName != "", fmt.Sprintf("%s.%s", mv.SchemaName, mv.ViewName), mv.ViewName) + return utils.GetObjectName(mv.SchemaName, mv.ViewName) } func (mv *MView) GetSchemaName() string { return mv.SchemaName } diff --git a/yb-voyager/src/query/queryparser/helpers_protomsg.go b/yb-voyager/src/query/queryparser/helpers_protomsg.go index 562aceac4..de3204f3c 100644 --- a/yb-voyager/src/query/queryparser/helpers_protomsg.go +++ b/yb-voyager/src/query/queryparser/helpers_protomsg.go @@ -420,18 +420,6 @@ func ProtoAsSelectStmt(msg protoreflect.Message) (*pg_query.SelectStmt, error) { return selectStmtNode, nil } -func GetConstraintNode(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") - } - constNode, ok := protoMsg.(*pg_query.Constraint) - if !ok { - return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_CONSTRAINT_NODE) - } - return constNode, nil -} - /* Example: options:{def_elem:{defname:"security_invoker" arg:{string:{sval:"true"}} defaction:DEFELEM_UNSPEC location:32}} diff --git a/yb-voyager/src/query/queryparser/helpers_struct.go b/yb-voyager/src/query/queryparser/helpers_struct.go index 37d421bdf..33b0dc759 100644 --- a/yb-voyager/src/query/queryparser/helpers_struct.go +++ b/yb-voyager/src/query/queryparser/helpers_struct.go @@ -20,7 +20,8 @@ import ( "strings" pg_query "github.com/pganalyze/pg_query_go/v6" - "github.com/samber/lo" + + "github.com/yugabyte/yb-voyager/yb-voyager/src/utils" ) const ( @@ -68,7 +69,7 @@ func GetObjectTypeAndObjectName(parseTree *pg_query.ParseResult) (string, string } funcNameList := stmt.GetFuncname() funcSchemaName, funcName := getFunctionObjectName(funcNameList) - return objectType, lo.Ternary(funcSchemaName != "", fmt.Sprintf("%s.%s", funcSchemaName, funcName), funcName) + return objectType, utils.GetObjectName(funcSchemaName, funcName) case isViewStmt: viewName := viewNode.ViewStmt.View return "VIEW", getObjectNameFromRangeVar(viewName) @@ -83,7 +84,7 @@ func GetObjectTypeAndObjectName(parseTree *pg_query.ParseResult) (string, string indexName := createIndexNode.IndexStmt.Idxname schemaName := createIndexNode.IndexStmt.Relation.GetSchemaname() tableName := createIndexNode.IndexStmt.Relation.GetRelname() - fullyQualifiedName := lo.Ternary(schemaName != "", schemaName+"."+tableName, tableName) + fullyQualifiedName := utils.GetObjectName(schemaName, tableName) displayObjName := fmt.Sprintf("%s ON %s", indexName, fullyQualifiedName) return "INDEX", displayObjName default: @@ -99,7 +100,7 @@ func isArrayType(typeName *pg_query.TypeName) bool { func getObjectNameFromRangeVar(obj *pg_query.RangeVar) string { schema := obj.Schemaname name := obj.Relname - return lo.Ternary(schema != "", fmt.Sprintf("%s.%s", schema, name), name) + return utils.GetObjectName(schema, name) } func getFunctionObjectName(funcNameList []*pg_query.Node) (string, string) { diff --git a/yb-voyager/src/query/queryparser/object_collector.go b/yb-voyager/src/query/queryparser/object_collector.go index 4da468265..acb06d768 100644 --- a/yb-voyager/src/query/queryparser/object_collector.go +++ b/yb-voyager/src/query/queryparser/object_collector.go @@ -20,8 +20,10 @@ import ( "github.com/samber/lo" log "github.com/sirupsen/logrus" - "github.com/yugabyte/yb-voyager/yb-voyager/src/constants" "google.golang.org/protobuf/reflect/protoreflect" + + "github.com/yugabyte/yb-voyager/yb-voyager/src/constants" + "github.com/yugabyte/yb-voyager/yb-voyager/src/utils" ) // ObjectPredicate defines a function signature that decides whether to include an object. @@ -76,7 +78,7 @@ func (c *ObjectCollector) Collect(msg protoreflect.Message) { case PG_QUERY_RANGEVAR_NODE: schemaName := GetStringField(msg, "schemaname") relName := GetStringField(msg, "relname") - objectName := lo.Ternary(schemaName != "", schemaName+"."+relName, relName) + objectName := utils.GetObjectName(schemaName, relName) log.Debugf("[RangeVar] fetched schemaname=%s relname=%s objectname=%s field\n", schemaName, relName, objectName) // it will be either table or view, considering objectType=table for both if c.predicate(schemaName, relName, constants.TABLE) { @@ -89,7 +91,7 @@ func (c *ObjectCollector) Collect(msg protoreflect.Message) { if relationMsg != nil { schemaName := GetStringField(relationMsg, "schemaname") relName := GetStringField(relationMsg, "relname") - objectName := lo.Ternary(schemaName != "", schemaName+"."+relName, relName) + objectName := utils.GetObjectName(schemaName, relName) log.Debugf("[IUD] fetched schemaname=%s relname=%s objectname=%s field\n", schemaName, relName, objectName) if c.predicate(schemaName, relName, "table") { c.addObject(objectName) @@ -103,7 +105,7 @@ func (c *ObjectCollector) Collect(msg protoreflect.Message) { return } - objectName := lo.Ternary(schemaName != "", schemaName+"."+functionName, functionName) + objectName := utils.GetObjectName(schemaName, functionName) log.Debugf("[Funccall] fetched schemaname=%s objectname=%s field\n", schemaName, objectName) if c.predicate(schemaName, functionName, constants.FUNCTION) { c.addObject(objectName) diff --git a/yb-voyager/src/utils/utils.go b/yb-voyager/src/utils/utils.go index ecf178a57..7536d23cc 100644 --- a/yb-voyager/src/utils/utils.go +++ b/yb-voyager/src/utils/utils.go @@ -740,3 +740,7 @@ func CheckTools(tools ...string) []string { return missingTools } + +func GetObjectName(schemaName, objName string) string { + return lo.Ternary(schemaName != "", schemaName+"."+objName, objName) +} From c6b0f80eb5be2e74d084815f7d7f1d4c09f26b77 Mon Sep 17 00:00:00 2001 From: priyanshi-yb Date: Mon, 6 Jan 2025 16:47:22 +0530 Subject: [PATCH 9/9] nit --- yb-voyager/cmd/exportSchema.go | 4 +-- .../src/query/queryparser/ddl_processor.go | 28 +++++++++---------- .../src/query/queryparser/helpers_struct.go | 6 ++-- .../src/query/queryparser/object_collector.go | 6 ++-- yb-voyager/src/utils/utils.go | 2 +- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/yb-voyager/cmd/exportSchema.go b/yb-voyager/cmd/exportSchema.go index c202a5523..f84888a8f 100644 --- a/yb-voyager/cmd/exportSchema.go +++ b/yb-voyager/cmd/exportSchema.go @@ -95,7 +95,7 @@ func exportSchema() error { log.Errorf("failed to get migration UUID: %v", err) return fmt.Errorf("failed to get migration UUID: %w", err) } - + utils.PrintAndLog("export of schema for source type as '%s'\n", source.DBType) // Check connection with source database. err = source.DB().Connect() @@ -467,7 +467,7 @@ func applyShardingRecommendationIfMatching(sqlInfo *sqlInfo, shardedTables []str } // true -> oracle, false -> PG - parsedObjectName := utils.GetObjectName(relation.Schemaname, relation.Relname) + parsedObjectName := utils.BuildObjectName(relation.Schemaname, relation.Relname) match := false switch source.DBType { diff --git a/yb-voyager/src/query/queryparser/ddl_processor.go b/yb-voyager/src/query/queryparser/ddl_processor.go index 469353f10..4bc488f79 100644 --- a/yb-voyager/src/query/queryparser/ddl_processor.go +++ b/yb-voyager/src/query/queryparser/ddl_processor.go @@ -312,7 +312,7 @@ type TableColumn struct { } func (tc *TableColumn) GetFullTypeName() string { - return utils.GetObjectName(tc.TypeSchema, tc.TypeName) + return utils.BuildObjectName(tc.TypeSchema, tc.TypeName) } type TableConstraint struct { @@ -346,7 +346,7 @@ func (c *TableConstraint) generateConstraintName(tableName string) string { } func (t *Table) GetObjectName() string { - return utils.GetObjectName(t.SchemaName, t.TableName) + return utils.BuildObjectName(t.SchemaName, t.TableName) } func (t *Table) GetSchemaName() string { return t.SchemaName } @@ -381,7 +381,7 @@ func (t *Table) addConstraint(conType pg_query.ConstrType, columns []string, spe conName := lo.Ternary(specifiedConName == "", generatedConName, specifiedConName) tc.ConstraintName = conName if conType == FOREIGN_CONSTR_TYPE { - tc.ReferencedTable = utils.GetObjectName(referencedTable.Schemaname, referencedTable.Relname) + tc.ReferencedTable = utils.BuildObjectName(referencedTable.Schemaname, referencedTable.Relname) } t.Constraints = append(t.Constraints, tc) } @@ -433,7 +433,7 @@ type ForeignTable struct { } func (f *ForeignTable) GetObjectName() string { - return utils.GetObjectName(f.SchemaName, f.TableName) + return utils.BuildObjectName(f.SchemaName, f.TableName) } func (f *ForeignTable) GetSchemaName() string { return f.SchemaName } @@ -524,7 +524,7 @@ type IndexParam struct { } func (indexParam *IndexParam) GetFullExprCastTypeName() string { - return utils.GetObjectName(indexParam.ExprCastTypeSchema, indexParam.ExprCastTypeName) + return utils.BuildObjectName(indexParam.ExprCastTypeSchema, indexParam.ExprCastTypeName) } func (i *Index) GetObjectName() string { @@ -533,7 +533,7 @@ func (i *Index) GetObjectName() string { func (i *Index) GetSchemaName() string { return i.SchemaName } func (i *Index) GetTableName() string { - return utils.GetObjectName(i.SchemaName, i.TableName) + return utils.BuildObjectName(i.SchemaName, i.TableName) } func (i *Index) GetObjectType() string { return INDEX_OBJECT_TYPE } @@ -611,7 +611,7 @@ func (atProcessor *AlterTableProcessor) Process(parseTree *pg_query.ParseResult) In case of FKs the reference table is in PKTable field and columns are in FkAttrs */ alter.ConstraintColumns = parseColumnsFromKeys(constraint.FkAttrs) - alter.ConstraintReferencedTable = utils.GetObjectName(constraint.Pktable.Schemaname, constraint.Pktable.Relname) + alter.ConstraintReferencedTable = utils.BuildObjectName(constraint.Pktable.Schemaname, constraint.Pktable.Relname) } case pg_query.AlterTableType_AT_DisableRule: @@ -651,7 +651,7 @@ type AlterTable struct { } func (a *AlterTable) GetObjectName() string { - return utils.GetObjectName(a.SchemaName, a.TableName) + return utils.BuildObjectName(a.SchemaName, a.TableName) } func (a *AlterTable) GetSchemaName() string { return a.SchemaName } @@ -716,7 +716,7 @@ type Policy struct { } func (p *Policy) GetObjectName() string { - qualifiedTable := utils.GetObjectName(p.SchemaName, p.TableName) + qualifiedTable := utils.BuildObjectName(p.SchemaName, p.TableName) return fmt.Sprintf("%s ON %s", p.PolicyName, qualifiedTable) } func (p *Policy) GetSchemaName() string { return p.SchemaName } @@ -792,7 +792,7 @@ func (t *Trigger) GetObjectName() string { } func (t *Trigger) GetTableName() string { - return utils.GetObjectName(t.SchemaName, t.TableName) + return utils.BuildObjectName(t.SchemaName, t.TableName) } func (t *Trigger) GetSchemaName() string { return t.SchemaName } @@ -869,7 +869,7 @@ type CreateType struct { } func (c *CreateType) GetObjectName() string { - return utils.GetObjectName(c.SchemaName, c.TypeName) + return utils.BuildObjectName(c.SchemaName, c.TypeName) } func (c *CreateType) GetSchemaName() string { return c.SchemaName } @@ -891,7 +891,7 @@ func (v *ViewProcessor) Process(parseTree *pg_query.ParseResult) (DDLObject, err viewSchemaName := viewNode.ViewStmt.View.Schemaname viewName := viewNode.ViewStmt.View.Relname - qualifiedViewName := utils.GetObjectName(viewSchemaName, viewName) + qualifiedViewName := utils.BuildObjectName(viewSchemaName, viewName) /* view_stmt:{view:{schemaname:"public" relname:"invoker_view" inh:true relpersistence:"p" location:12} @@ -924,7 +924,7 @@ type View struct { } func (v *View) GetObjectName() string { - return utils.GetObjectName(v.SchemaName, v.ViewName) + return utils.BuildObjectName(v.SchemaName, v.ViewName) } func (v *View) GetSchemaName() string { return v.SchemaName } @@ -956,7 +956,7 @@ type MView struct { } func (mv *MView) GetObjectName() string { - return utils.GetObjectName(mv.SchemaName, mv.ViewName) + return utils.BuildObjectName(mv.SchemaName, mv.ViewName) } func (mv *MView) GetSchemaName() string { return mv.SchemaName } diff --git a/yb-voyager/src/query/queryparser/helpers_struct.go b/yb-voyager/src/query/queryparser/helpers_struct.go index 33b0dc759..c3128de6b 100644 --- a/yb-voyager/src/query/queryparser/helpers_struct.go +++ b/yb-voyager/src/query/queryparser/helpers_struct.go @@ -69,7 +69,7 @@ func GetObjectTypeAndObjectName(parseTree *pg_query.ParseResult) (string, string } funcNameList := stmt.GetFuncname() funcSchemaName, funcName := getFunctionObjectName(funcNameList) - return objectType, utils.GetObjectName(funcSchemaName, funcName) + return objectType, utils.BuildObjectName(funcSchemaName, funcName) case isViewStmt: viewName := viewNode.ViewStmt.View return "VIEW", getObjectNameFromRangeVar(viewName) @@ -84,7 +84,7 @@ func GetObjectTypeAndObjectName(parseTree *pg_query.ParseResult) (string, string indexName := createIndexNode.IndexStmt.Idxname schemaName := createIndexNode.IndexStmt.Relation.GetSchemaname() tableName := createIndexNode.IndexStmt.Relation.GetRelname() - fullyQualifiedName := utils.GetObjectName(schemaName, tableName) + fullyQualifiedName := utils.BuildObjectName(schemaName, tableName) displayObjName := fmt.Sprintf("%s ON %s", indexName, fullyQualifiedName) return "INDEX", displayObjName default: @@ -100,7 +100,7 @@ func isArrayType(typeName *pg_query.TypeName) bool { func getObjectNameFromRangeVar(obj *pg_query.RangeVar) string { schema := obj.Schemaname name := obj.Relname - return utils.GetObjectName(schema, name) + return utils.BuildObjectName(schema, name) } func getFunctionObjectName(funcNameList []*pg_query.Node) (string, string) { diff --git a/yb-voyager/src/query/queryparser/object_collector.go b/yb-voyager/src/query/queryparser/object_collector.go index acb06d768..ba7561a1b 100644 --- a/yb-voyager/src/query/queryparser/object_collector.go +++ b/yb-voyager/src/query/queryparser/object_collector.go @@ -78,7 +78,7 @@ func (c *ObjectCollector) Collect(msg protoreflect.Message) { case PG_QUERY_RANGEVAR_NODE: schemaName := GetStringField(msg, "schemaname") relName := GetStringField(msg, "relname") - objectName := utils.GetObjectName(schemaName, relName) + objectName := utils.BuildObjectName(schemaName, relName) log.Debugf("[RangeVar] fetched schemaname=%s relname=%s objectname=%s field\n", schemaName, relName, objectName) // it will be either table or view, considering objectType=table for both if c.predicate(schemaName, relName, constants.TABLE) { @@ -91,7 +91,7 @@ func (c *ObjectCollector) Collect(msg protoreflect.Message) { if relationMsg != nil { schemaName := GetStringField(relationMsg, "schemaname") relName := GetStringField(relationMsg, "relname") - objectName := utils.GetObjectName(schemaName, relName) + objectName := utils.BuildObjectName(schemaName, relName) log.Debugf("[IUD] fetched schemaname=%s relname=%s objectname=%s field\n", schemaName, relName, objectName) if c.predicate(schemaName, relName, "table") { c.addObject(objectName) @@ -105,7 +105,7 @@ func (c *ObjectCollector) Collect(msg protoreflect.Message) { return } - objectName := utils.GetObjectName(schemaName, functionName) + objectName := utils.BuildObjectName(schemaName, functionName) log.Debugf("[Funccall] fetched schemaname=%s objectname=%s field\n", schemaName, objectName) if c.predicate(schemaName, functionName, constants.FUNCTION) { c.addObject(objectName) diff --git a/yb-voyager/src/utils/utils.go b/yb-voyager/src/utils/utils.go index 7536d23cc..b168d886c 100644 --- a/yb-voyager/src/utils/utils.go +++ b/yb-voyager/src/utils/utils.go @@ -741,6 +741,6 @@ func CheckTools(tools ...string) []string { return missingTools } -func GetObjectName(schemaName, objName string) string { +func BuildObjectName(schemaName, objName string) string { return lo.Ternary(schemaName != "", schemaName+"."+objName, objName) }