Skip to content

Commit

Permalink
Made suggested changes
Browse files Browse the repository at this point in the history
  • Loading branch information
ShivanshGahlot committed Jan 6, 2025
1 parent e59cd44 commit 34668fd
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 49 deletions.
4 changes: 2 additions & 2 deletions migtests/tests/analyze-schema/summary.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
},
{
"ObjectType": "TABLE",
"TotalCount": 61,
"TotalCount": 62,
"InvalidCount": 52,
"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, bigint_multirange_table, date_multirange_table, int_multirange_table, numeric_multirange_table, timestamp_multirange_table, timestamptz_multirange_table, users_unique_nulls_not_distinct, sales_unique_nulls_not_distinct, users_unique_nulls_distinct, sales_unique_nulls_not_distinct_alter" },
"ObjectNames": "test_table_in_type_file, sales_data, salaries2, sales, test_1, test_2, test_non_pk_multi_column_list, test_3, test_4, test_5, test_6, test_7, test_8, test_9, order_details, public.employees4, enum_example.bugs, table_xyz, table_abc, table_1, table_test, test_interval, public.range_columns_partition_test, public.range_columns_partition_test_copy, anydata_test, anydataset_test, anytype_test, uritype_test, \"Test\", public.meeting, public.pr, public.foreign_def_test, public.users, foreign_def_test1, foreign_def_test2, unique_def_test, unique_def_test1, test_xml_type, test_xid_type, public.test_jsonb, public.inet_type, public.citext_type, public.documents, public.ts_query_table, combined_tbl, combined_tbl1, test_udt, test_arr_enum, public.locations, public.xml_data_example, image, employees, bigint_multirange_table, date_multirange_table, int_multirange_table, numeric_multirange_table, timestamp_multirange_table, timestamptz_multirange_table, users_unique_nulls_distinct, users_unique_nulls_not_distinct, sales_unique_nulls_not_distinct, sales_unique_nulls_not_distinct_alter" },
{
"ObjectType": "INDEX",
"TotalCount": 43,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@
},
{
"ObjectType": "TABLE",
"TotalCount": 88,
"TotalCount": 92,
"InvalidCount": 41,
"ObjectNames": "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.sales_unique_nulls_not_distinct, public.sales_unique_nulls_not_distinct_alter, 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.users_unique_nulls_distinct, public.users_unique_nulls_not_distinct, 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.sales_unique_nulls_not_distinct, schema2.sales_unique_nulls_not_distinct_alter, 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.users_unique_nulls_distinct, schema2.users_unique_nulls_not_distinct, schema2.with_example1, schema2.with_example2, test_views.view_table1, test_views.view_table2"
"ObjectNames": "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.employeesforview, 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.sales_unique_nulls_not_distinct, public.sales_unique_nulls_not_distinct_alter, 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.users_unique_nulls_distinct, public.users_unique_nulls_not_distinct, public.users_unique_nulls_not_distinct_index, 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.employeesforview, 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.sales_unique_nulls_not_distinct, schema2.sales_unique_nulls_not_distinct_alter, 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.users_unique_nulls_distinct, schema2.users_unique_nulls_not_distinct, schema2.users_unique_nulls_not_distinct_index, schema2.with_example1, schema2.with_example2, test_views.view_table1, test_views.view_table2"
},
{
"ObjectType": "INDEX",
"TotalCount": 26,
"InvalidCount": 22,
"ObjectNames": "idx8 ON public.combined_tbl, idx9 ON public.combined_tbl, idx1 ON public.combined_tbl, idx2 ON public.combined_tbl, idx3 ON public.combined_tbl, idx4 ON public.combined_tbl, idx5 ON public.combined_tbl, idx6 ON public.combined_tbl, idx7 ON public.combined_tbl, idx_array ON public.documents, idx_box_data ON public.mixed_data_types_table1, idx_box_data_brin ON public.mixed_data_types_table1, idx_citext ON public.citext_type, idx_citext1 ON public.citext_type, idx_citext2 ON public.citext_type, idx_inet ON public.inet_type, idx_inet1 ON public.inet_type, idx_json ON public.test_jsonb, idx_json2 ON public.test_jsonb, idx_point_data ON public.mixed_data_types_table1, idx_valid ON public.test_jsonb, tsquery_idx ON public.ts_query_table, tsvector_idx ON public.documents, idx_box_data ON schema2.mixed_data_types_table1, idx_box_data_spgist ON schema2.mixed_data_types_table1, idx_point_data ON schema2.mixed_data_types_table1"
"TotalCount": 28,
"InvalidCount": 24,
"ObjectNames": "idx1 ON public.combined_tbl, idx2 ON public.combined_tbl, idx3 ON public.combined_tbl, idx4 ON public.combined_tbl, idx5 ON public.combined_tbl, idx6 ON public.combined_tbl, idx7 ON public.combined_tbl, idx8 ON public.combined_tbl, idx9 ON public.combined_tbl, idx_array ON public.documents, idx_box_data ON public.mixed_data_types_table1, idx_box_data ON schema2.mixed_data_types_table1, idx_box_data_brin ON public.mixed_data_types_table1, idx_box_data_spgist ON schema2.mixed_data_types_table1, idx_citext ON public.citext_type, idx_citext1 ON public.citext_type, idx_citext2 ON public.citext_type, idx_inet ON public.inet_type, idx_inet1 ON public.inet_type, idx_json ON public.test_jsonb, idx_json2 ON public.test_jsonb, idx_point_data ON public.mixed_data_types_table1, idx_point_data ON schema2.mixed_data_types_table1, idx_valid ON public.test_jsonb, tsquery_idx ON public.ts_query_table, tsvector_idx ON public.documents, users_unique_nulls_not_distinct_index_email ON public.users_unique_nulls_not_distinct_index, users_unique_nulls_not_distinct_index_email ON schema2.users_unique_nulls_not_distinct_index"
},
{
"ObjectType": "FUNCTION",
Expand All @@ -75,7 +75,7 @@
"ObjectType": "VIEW",
"TotalCount": 10,
"InvalidCount": 6,
"ObjectNames": "public.ordersentry_view, public.sales_employees, schema2.sales_employees, test_views.v1, test_views.v2, test_views.v3, test_views.v4, public.view_explicit_security_invoker, schema2.top_employees_view, public.top_employees_view"
"ObjectNames": "public.ordersentry_view, public.sales_employees, public.top_employees_view, public.view_explicit_security_invoker, schema2.sales_employees, schema2.top_employees_view, test_views.v1, test_views.v2, test_views.v3, test_views.v4"
},
{
"ObjectType": "TRIGGER",
Expand Down Expand Up @@ -189,13 +189,15 @@
"schema2.users_unique_nulls_distinct",
"schema2.users_unique_nulls_not_distinct",
"schema2.sales_unique_nulls_not_distinct",
"public.users_unique_nulls_not_distinct_index",
"schema2.users_unique_nulls_not_distinct_index",
"schema2.sales_unique_nulls_not_distinct_alter",
"public.users_unique_nulls_distinct",
"public.users_unique_nulls_not_distinct",
"public.sales_unique_nulls_not_distinct",
"public.sales_unique_nulls_not_distinct_alter"
],
"ColocatedReasoning": "Recommended instance type with 4 vCPU and 16 GiB memory could fit 102 objects (86 tables/materialized views and 16 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 108 objects (90 tables/materialized views and 18 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",
Expand Down Expand Up @@ -671,14 +673,22 @@
"Objects": [
{
"ObjectName": "public.view_explicit_security_invoker",
"SqlStatement": "CREATE VIEW public.view_explicit_security_invoker WITH (security_invoker='true') AS\n SELECT employees.employee_id,\n employees.first_name\n FROM public.employees;"
"SqlStatement": "CREATE VIEW public.view_explicit_security_invoker WITH (security_invoker='true') AS\n SELECT employee_id,\n first_name\n FROM public.employees;"
}
],
"MinimumVersionsFixedIn": null
},
{
"FeatureName": "Unique Nulls Not Distinct",
"Objects": [
{
"ObjectName": "users_unique_nulls_not_distinct_index_email ON public.users_unique_nulls_not_distinct_index",
"SqlStatement": "CREATE UNIQUE INDEX users_unique_nulls_not_distinct_index_email ON public.users_unique_nulls_not_distinct_index USING btree (email) NULLS NOT DISTINCT;"
},
{
"ObjectName": "users_unique_nulls_not_distinct_index_email ON schema2.users_unique_nulls_not_distinct_index",
"SqlStatement": "CREATE UNIQUE INDEX users_unique_nulls_not_distinct_index_email ON schema2.users_unique_nulls_not_distinct_index USING btree (email) NULLS NOT DISTINCT;"
},
{
"ObjectName": "public.sales_unique_nulls_not_distinct",
"SqlStatement": "ALTER TABLE ONLY public.sales_unique_nulls_not_distinct\n ADD CONSTRAINT sales_unique_nulls_not_distin_store_id_product_id_sale_date_key UNIQUE NULLS NOT DISTINCT (store_id, product_id, sale_date);"
Expand Down Expand Up @@ -2556,8 +2566,63 @@
"ObjectType": "",
"ParentTableName": null,
"SizeInBytes": 0
},
{
"SchemaName": "public",
"ObjectName": "users_unique_nulls_not_distinct_index",
"RowCount": 0,
"ColumnCount": 2,
"Reads": 0,
"Writes": 0,
"ReadsPerSecond": 0,
"WritesPerSecond": 0,
"IsIndex": false,
"ObjectType": "",
"ParentTableName": null,
"SizeInBytes": 0
},
{
"SchemaName": "public",
"ObjectName": "users_unique_nulls_not_distinct_index_email",
"RowCount": null,
"ColumnCount": 1,
"Reads": 0,
"Writes": 0,
"ReadsPerSecond": 0,
"WritesPerSecond": 0,
"IsIndex": true,
"ObjectType": "",
"ParentTableName": "public.users_unique_nulls_not_distinct_index",
"SizeInBytes": 8192
},
{
"SchemaName": "schema2",
"ObjectName": "users_unique_nulls_not_distinct_index",
"RowCount": 0,
"ColumnCount": 2,
"Reads": 0,
"Writes": 0,
"ReadsPerSecond": 0,
"WritesPerSecond": 0,
"IsIndex": false,
"ObjectType": "",
"ParentTableName": null,
"SizeInBytes": 0
},
{
"SchemaName": "schema2",
"ObjectName": "users_unique_nulls_not_distinct_index_email",
"RowCount": null,
"ColumnCount": 1,
"Reads": 0,
"Writes": 0,
"ReadsPerSecond": 0,
"WritesPerSecond": 0,
"IsIndex": true,
"ObjectType": "",
"ParentTableName": "schema2.users_unique_nulls_not_distinct_index",
"SizeInBytes": 8192
}

],
"Notes": [
"There are some Unlogged tables in the schema. They will be created as regular LOGGED tables in YugabyteDB as unlogged tables are not supported."
Expand Down
10 changes: 10 additions & 0 deletions migtests/tests/pg/assessment-report-test/pg_assessment_report.sql
Original file line number Diff line number Diff line change
Expand Up @@ -459,4 +459,14 @@ CREATE TABLE sales_unique_nulls_not_distinct_alter (
ALTER TABLE sales_unique_nulls_not_distinct_alter
ADD CONSTRAINT sales_unique_nulls_not_distinct_alter_unique UNIQUE NULLS NOT DISTINCT (store_id, product_id, sale_date);

-- Create a unique index on a column with NULLs with the NULLS NOT DISTINCT option
CREATE TABLE users_unique_nulls_not_distinct_index (
id INTEGER PRIMARY KEY,
email TEXT
);

CREATE UNIQUE INDEX users_unique_nulls_not_distinct_index_email
ON users_unique_nulls_not_distinct_index (email)
NULLS NOT DISTINCT;


30 changes: 14 additions & 16 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import (
"slices"

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"
Expand Down Expand Up @@ -404,24 +406,20 @@ func NewUniqueNullsNotDistinctDetector(query string) *UniqueNullsNotDistinctDete

// Detect checks if a unique constraint is defined which has nulls not distinct
func (d *UniqueNullsNotDistinctDetector) Detect(msg protoreflect.Message) error {
// If message is of type PG_QUERY_TABLECONSTRAINT_NODE
if !(queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_TABLECONSTRAINT_NODE) {
return nil
}

// Fetch contype as an enum number
constraintType := queryparser.GetEnumNumField(msg, "contype")
constraintTypeStr := queryparser.GetResolvedEnumName(msg, "contype", constraintType)
if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_INDEX_STMT_NODE {
proto := msg.Interface().(proto.Message)
indexStmt := proto.(*pg_query.IndexStmt)

// Check if the constraint is of type CONSTR_UNIQUE
if constraintTypeStr != "CONSTR_UNIQUE" {
return nil
}
if indexStmt.Unique && indexStmt.NullsNotDistinct {
d.detected = true
}
} else if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_TABLECONSTRAINT_NODE {
proto := msg.Interface().(proto.Message)
constraintNode := proto.(*pg_query.Constraint)

// Check if the constraint has nulls not distinct
nullsNotDistinct := queryparser.GetBoolField(msg, "nulls_not_distinct")
if nullsNotDistinct {
d.detected = true
if constraintNode.Contype == pg_query.ConstrType_CONSTR_UNIQUE && constraintNode.NullsNotDistinct {
d.detected = true
}
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ CHECK (xpath_exists('/invoice/customer', data));`
UNIQUE NULLS NOT DISTINCT (product_name, serial_number)
);`
stmt22 = `ALTER TABLE public.products ADD CONSTRAINT unique_product_name UNIQUE NULLS NOT DISTINCT (product_name);`
stmt23 = `CREATE UNIQUE INDEX unique_email_idx ON users (email) NULLS NOT DISTINCT;`
)

func modifiedIssuesforPLPGSQL(issues []QueryIssue, objType string, objName string) []QueryIssue {
Expand Down Expand Up @@ -299,6 +300,9 @@ func TestDDLIssues(t *testing.T) {
stmt22: []QueryIssue{
NewUniqueNullsNotDistinctIssue("TABLE", "public.products", stmt22),
},
stmt23: []QueryIssue{
NewUniqueNullsNotDistinctIssue("INDEX", "unique_email_idx ON users", stmt23),
},
}
for _, stmt := range requiredDDLs {
err := parserIssueDetector.ParseRequiredDDLs(stmt)
Expand All @@ -313,7 +317,7 @@ func TestDDLIssues(t *testing.T) {
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)
assert.True(t, found, "Expected issue not found: %v in statement: %s. \nFound: %v", expectedIssue, stmt, issues)
}
}
}
Expand Down
21 changes: 0 additions & 21 deletions yb-voyager/src/query/queryparser/helpers_protomsg.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,27 +395,6 @@ func GetEnumNumField(msg protoreflect.Message, fieldName string) protoreflect.En
return 0
}

// GetResolvedEnumName resolves the enum value to its symbolic name.
func GetResolvedEnumName(msg protoreflect.Message, fieldName string, enumNum protoreflect.EnumNumber) string {
fieldDescriptor := msg.Descriptor().Fields().ByName(protoreflect.Name(fieldName))
if fieldDescriptor == nil {
return "UNKNOWN"
}

enumDescriptor := fieldDescriptor.Enum()
if enumDescriptor == nil {
return "UNKNOWN"
}

// Resolve the enum value to its symbolic name
resolvedName := enumDescriptor.Values().ByNumber(protoreflect.EnumNumber(enumNum)).Name()

if resolvedName == "" {
return "UNKNOWN"
}
return string(resolvedName)
}

// GetSchemaAndObjectName extracts the schema and object name from a list.
func GetSchemaAndObjectName(nameList protoreflect.List) (string, string) {
var schemaName, objectName string
Expand Down
1 change: 1 addition & 0 deletions yb-voyager/src/query/queryparser/traversal_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const (
PG_QUERY_COPYSTSMT_NODE = "pg_query.CopyStmt"

PG_QUERY_TABLECONSTRAINT_NODE = "pg_query.Constraint"
PG_QUERY_INDEX_STMT_NODE = "pg_query.IndexStmt"
)

// function type for processing nodes during traversal
Expand Down

0 comments on commit 34668fd

Please sign in to comment.