Skip to content

Commit

Permalink
Reporting Foreign Key references Partitioned table in analyze and ass…
Browse files Browse the repository at this point in the history
…essment (#2145)
  • Loading branch information
priyanshi-yb authored Jan 6, 2025
1 parent 16b08fe commit 5a50ec5
Show file tree
Hide file tree
Showing 17 changed files with 239 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
14 changes: 12 additions & 2 deletions migtests/tests/analyze-schema/expected_issues.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,16 @@
"GH": "",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "TABLE",
"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 ",
"GH": "",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "MVIEW",
Expand Down Expand Up @@ -466,7 +476,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",
Expand Down Expand Up @@ -1515,7 +1525,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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
{
Expand Down Expand Up @@ -634,6 +634,16 @@
],
"MinimumVersionsFixedIn": null
},
{
"FeatureName": "Foreign key constraint references partitioned table",
"Objects": [
{
"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);"
}
],
"MinimumVersionsFixedIn": null
},
{
"FeatureName": "Regex Functions",
"Objects": [
Expand Down Expand Up @@ -1011,7 +1021,7 @@
"SchemaName": "public",
"ObjectName": "test_jsonb",
"RowCount": 0,
"ColumnCount": 3,
"ColumnCount": 4,
"Reads": 0,
"Writes": 0,
"ReadsPerSecond": 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
1 change: 1 addition & 0 deletions yb-voyager/cmd/analyzeSchema.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,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:
Expand Down
1 change: 1 addition & 0 deletions yb-voyager/cmd/assessMigrationCommand.go
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,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, ""))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.JSON_TYPE_PREDICATE_NAME, "", queryissue.JSON_TYPE_PREDICATE, schemaAnalysisReport, false, ""))

return lo.Filter(unsupportedFeatures, func(f UnsupportedFeature, _ int) bool {
Expand Down
5 changes: 2 additions & 3 deletions yb-voyager/cmd/exportSchema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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.BuildObjectName(relation.Schemaname, relation.Relname)

match := false
switch source.DBType {
Expand Down
3 changes: 3 additions & 0 deletions yb-voyager/src/query/queryissue/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,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
Expand Down
20 changes: 20 additions & 0 deletions yb-voyager/src/query/queryissue/detectors_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()]
Expand Down Expand Up @@ -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(
Expand Down
15 changes: 15 additions & 0 deletions yb-voyager/src/query/queryissue/issues_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,3 +465,18 @@ 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: "No workaround available ",
GH: "", // TODO
DocsLink: "", // TODO
}

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)
}
18 changes: 18 additions & 0 deletions yb-voyager/src/query/queryissue/issues_ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
53 changes: 53 additions & 0 deletions yb-voyager/src/query/queryissue/parser_issue_detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -841,3 +841,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{
NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt1, "abc_fk_abc_id_fkey"),
},
stmt2: []QueryIssue{
NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk1", stmt2, "fk"),
},
stmt3: []QueryIssue{
NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "abc_fk", stmt3, "fk_abc"),
},
stmt4: []QueryIssue{
NewForeignKeyReferencesPartitionedTableIssue(TABLE_OBJECT_TYPE, "schema1.abc_fk", stmt4, "abc_fk_abc_id_fkey"),
},
}
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)
}
}
}
Loading

0 comments on commit 5a50ec5

Please sign in to comment.