Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reporting Collation - deterministic attribute and Merge statement #2154

Merged
merged 12 commits into from
Jan 9, 2025
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@

--dropping multiple object
DROP COLLATION IF EXISTS coll1,coll2,coll3;
DROP COLLATION IF EXISTS coll1,coll2,coll3;

CREATE COLLATION special1 (provider = icu, locale = 'en@colCaseFirst=upper;colReorder=grek-latn', deterministic = true);

CREATE COLLATION ignore_accents (provider = icu, locale = 'und-u-ks-level1-kc-true', deterministic = false);

CREATE COLLATION schema2.upperfirst (provider = icu, locale = 'en-u-kf-upper');
20 changes: 20 additions & 0 deletions migtests/tests/analyze-schema/expected_issues.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,26 @@
"GH": "",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "COLLATION",
"ObjectName": "special1",
"Reason": "Deterministic attribute in collation",
"SqlStatement": "CREATE COLLATION special1 (provider = icu, locale = 'en@colCaseFirst=upper;colReorder=grek-latn', deterministic = true);",
"Suggestion": "This feature is not supported in YugabyteDB yet",
"GH": "",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "COLLATION",
"ObjectName": "ignore_accents",
"Reason": "Deterministic attribute in collation",
"SqlStatement": "CREATE COLLATION ignore_accents (provider = icu, locale = 'und-u-ks-level1-kc-true', deterministic = false);",
"Suggestion": "This feature is not supported in YugabyteDB yet",
"GH": "",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "TABLE",
Expand Down
6 changes: 6 additions & 0 deletions migtests/tests/analyze-schema/summary.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
"InvalidCount": 1,
"ObjectNames": "hollywood"
},
{
"ObjectType": "COLLATION",
"TotalCount": 3,
"InvalidCount": 3,
"ObjectNames": "special1, ignore_accents, schema2.upperfirst"
},
{
"ObjectType": "EXTENSION",
"TotalCount": 7,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,16 @@
},
{
"ObjectType": "TABLE",
"TotalCount": 5,
"TotalCount": 7,
"InvalidCount": 2,
"ObjectNames": "analytics.metrics, sales.orders, sales.test_json_chk, sales.events, sales.json_data"
"ObjectNames": "analytics.metrics, sales.orders, sales.test_json_chk, sales.events, sales.json_data, sales.customer_account, sales.recent_transactions"
},
{
"ObjectType": "SEQUENCE",
"TotalCount": 1,
"InvalidCount":0,
"ObjectNames": "sales.recent_transactions_transaction_id_seq"

},
{
"ObjectType": "VIEW",
Expand All @@ -48,11 +55,13 @@
"ColocatedTables": [
"sales.orders",
"analytics.metrics",
"sales.customer_account",
"sales.recent_transactions",
"sales.events",
"sales.json_data",
"sales.test_json_chk"
],
"ColocatedReasoning": "Recommended instance type with 4 vCPU and 16 GiB memory could fit 5 objects (5 tables/materialized views and 0 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec as colocated. 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 7 objects (7 tables/materialized views and 0 explicit/implicit indexes) with 0.00 MB size and throughput requirement of 0 reads/sec and 0 writes/sec as colocated. Non leaf partition tables/indexes and unsupported tables/indexes were not considered.",
"ShardedTables": null,
"NumNodes": 3,
"VCPUsPerInstance": 4,
Expand Down Expand Up @@ -115,6 +124,34 @@
],
"UnsupportedFeaturesDesc": "Features of the source database that are not supported on the target YugabyteDB.",
"TableIndexStats": [
{
"SchemaName": "sales",
"ObjectName": "customer_account",
"RowCount": 4,
"ColumnCount": 2,
"Reads": 7,
"Writes": 6,
"ReadsPerSecond": 0,
"WritesPerSecond": 0,
"IsIndex": false,
"ObjectType": "",
"ParentTableName": null,
"SizeInBytes": 8192
},
{
"SchemaName": "sales",
"ObjectName": "recent_transactions",
"RowCount": 3,
"ColumnCount": 3,
"Reads": 3,
"Writes": 3,
"ReadsPerSecond": 0,
"WritesPerSecond": 0,
"IsIndex": false,
"ObjectType": "",
"ParentTableName": null,
"SizeInBytes": 8192
},
{
"SchemaName": "sales",
"ObjectName": "test_json_chk",
Expand Down Expand Up @@ -214,6 +251,13 @@
"MinimumVersionsFixedIn": null
},
{
"ConstructTypeName": "Merge Statement",
"Query": "MERGE INTO sales.customer_account ca\nUSING sales.recent_transactions t \nON t.customer_id = ca.customer_id\nWHEN MATCHED THEN\n UPDATE SET balance = balance + transaction_value\nWHEN NOT MATCHED THEN\n INSERT (customer_id, balance)\n VALUES (t.customer_id, t.transaction_value)",
"DocsLink": "",
"MinimumVersionsFixedIn": null
},
{

"ConstructTypeName": "Jsonb Subscripting",
"Query": "SELECT \n data,\n data[$1] AS name, \n (data[$2]) as active\nFROM sales.test_json_chk",
"DocsLink": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,28 @@ create view sales.employ_depart_view AS SELECT
any_value(name) AS any_employee
FROM employees;

CREATE TABLE sales.customer_account (
customer_id INT PRIMARY KEY,
balance NUMERIC(10, 2) NOT NULL
);

INSERT INTO sales.customer_account (customer_id, balance)
VALUES
(1, 100.00),
(2, 200.00),
(3, 300.00);

CREATE TABLE sales.recent_transactions (
transaction_id SERIAL PRIMARY KEY,
customer_id INT NOT NULL,
transaction_value NUMERIC(10, 2) NOT NULL
);

INSERT INTO sales.recent_transactions (customer_id, transaction_value)
VALUES
(1, 50.00),
(3, -25.00),
(4, 150.00);
CREATE TABLE sales.test_json_chk (
id int,
name text,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ SELECT
any_value(name) AS any_employee
FROM employees;

MERGE INTO sales.customer_account ca
USING sales.recent_transactions t
ON t.customer_id = ca.customer_id
WHEN MATCHED THEN
UPDATE SET balance = balance + transaction_value
WHEN NOT MATCHED THEN
INSERT (customer_id, balance)
VALUES (t.customer_id, t.transaction_value);

select * from sales.customer_account ;
SELECT (sales.get_user_info(2))['name'] AS user_info;

SELECT (jsonb_build_object('name', 'PostgreSQL', 'version', 17, 'open_source', TRUE) || '{"key": "value2"}')['name'] AS json_obj;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@
"TotalCount": 4,
"InvalidCount": 2,
"ObjectNames": "policy_test_fine ON public.test_exclude_basic, policy_test_fine_2 ON public.employees2, policy_test_report ON public.test_xml_type, policy_test_report ON schema2.test_xml_type"
},
{
"ObjectType": "COLLATION",
"TotalCount": 2,
"InvalidCount": 1,
"ObjectNames": "schema2.ignore_accents, public.\"numeric\""
}
]
},
Expand Down Expand Up @@ -583,6 +589,16 @@
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#indexes-on-some-complex-data-types-are-not-supported",
"MinimumVersionsFixedIn": null
},
{
"FeatureName": "Deterministic attribute in collation",
"Objects": [
{
"ObjectName": "schema2.ignore_accents",
"SqlStatement": "CREATE COLLATION schema2.ignore_accents (provider = icu, deterministic = false, locale = 'und-u-kc-ks-level1');"
}
],
"MinimumVersionsFixedIn": null
},
{
"FeatureName": "Primary / Unique key constraints on complex datatypes",
"Objects": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,9 @@ WITH (security_invoker = true) AS
SELECT employee_id, first_name
FROM public.employees;

CREATE COLLATION schema2.ignore_accents (provider = icu, locale = 'und-u-ks-level1-kc-true', deterministic = false);

CREATE COLLATION public.numeric (provider = icu, locale = 'en@colNumeric=yes');
-- Testing tables with unique nulls not distinct constraints

-- Control case
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 @@ -1067,6 +1067,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.DETERMINISTIC_OPTION_WITH_COLLATION_NAME, "", queryissue.DETERMINISTIC_OPTION_WITH_COLLATION, schemaAnalysisReport, false, ""))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.UNIQUE_NULLS_NOT_DISTINCT_NAME, "", queryissue.UNIQUE_NULLS_NOT_DISTINCT, schemaAnalysisReport, false, ""))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.JSONB_SUBSCRIPTING_NAME, "", queryissue.JSONB_SUBSCRIPTING, schemaAnalysisReport, false, ""))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME, "", queryissue.FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE, schemaAnalysisReport, false, ""))
Expand Down
5 changes: 5 additions & 0 deletions yb-voyager/src/query/queryissue/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ const (
COPY_FROM_WHERE = "COPY FROM ... WHERE"
COPY_ON_ERROR = "COPY ... ON_ERROR"

DETERMINISTIC_OPTION_WITH_COLLATION = "DETERMINISTIC_OPTION_WITH_COLLATION"
DETERMINISTIC_OPTION_WITH_COLLATION_NAME = "Deterministic attribute in collation"
Comment on lines +82 to +83
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about naming the issue as Nondeterministic Collation

As per the docs - 'A collation is either deterministic or nondeterministic.'

Copy link
Contributor Author

@priyanshi-yb priyanshi-yb Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't rename the issue as Non-deterministic collation as we are not reporting the non-deterministic one only, we are reporting both the cases if deterministic or nondeterministic, as the deterministic option with collation is completely not supported in YB so it errors out if it is true even

CREATE COLLATION .... (..., deterministic = true);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh ok, what is the default if nothing is provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its Deterministic by default


MERGE_STATEMENT = "MERGE_STATEMENT"
MERGE_STATEMENT_NAME = "Merge Statement"
FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE = "FOREIGN_KEY_REFERENCED_PARTITIONED_TABLE"
FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME = "Foreign key constraint references partitioned table"

Expand Down
32 changes: 29 additions & 3 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func NewCopyCommandUnsupportedConstructsDetector(query string) *CopyCommandUnsup
// Detect if COPY command uses unsupported syntax i.e. COPY FROM ... WHERE and COPY... ON_ERROR
func (d *CopyCommandUnsupportedConstructsDetector) Detect(msg protoreflect.Message) error {
// Check if the message is a COPY statement
if msg.Descriptor().FullName() != queryparser.PG_QUERY_COPYSTSMT_NODE {
if msg.Descriptor().FullName() != queryparser.PG_QUERY_COPY_STMT_NODE {
return nil // Not a COPY statement, nothing to detect
}

Expand Down Expand Up @@ -453,6 +453,33 @@ func (d *JsonQueryFunctionDetector) GetIssues() []QueryIssue {
return issues
}

type MergeStatementDetector struct {
query string
isMergeStatementDetected bool
}

func NewMergeStatementDetector(query string) *MergeStatementDetector {
return &MergeStatementDetector{
query: query,
}
}

func (m *MergeStatementDetector) Detect(msg protoreflect.Message) error {
if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_MERGE_STMT_NODE {
m.isMergeStatementDetected = true
}
return nil

}

func (m *MergeStatementDetector) GetIssues() []QueryIssue {
var issues []QueryIssue
if m.isMergeStatementDetected {
issues = append(issues, NewMergeStatementIssue(DML_QUERY_OBJECT_TYPE, "", m.query))
}
return issues
}

type UniqueNullsNotDistinctDetector struct {
query string
detected bool
Expand All @@ -466,7 +493,7 @@ 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 queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_INDEXSTMT_NODE {
if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_INDEX_STMT_NODE {
indexStmt, err := queryparser.ProtoAsIndexStmt(msg)
if err != nil {
return err
Expand Down Expand Up @@ -507,7 +534,6 @@ func NewJsonPredicateExprDetector(query string) *JsonPredicateExprDetector {
query: query,
}
}

func (j *JsonPredicateExprDetector) Detect(msg protoreflect.Message) error {
if queryparser.GetMsgFullName(msg) == queryparser.PG_QUERY_JSON_IS_PREDICATE_NODE {
/*
Expand Down
23 changes: 23 additions & 0 deletions yb-voyager/src/query/queryissue/detectors_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,27 @@ func (v *MViewIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIss
return nil, nil
}

//===============COLLATION ISSUE DETECTOR ====================

type CollationIssueDetector struct{}

func (c *CollationIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIssue, error) {
collation, ok := obj.(*queryparser.Collation)
if !ok {
return nil, fmt.Errorf("invalid object type: expected Collation")
}
issues := make([]QueryIssue, 0)
if slices.Contains(collation.Options, "deterministic") {
// deterministic attribute is itself not supported in YB either true or false so checking only whether option is present or not
issues = append(issues, NewDeterministicOptionCollationIssue(
collation.GetObjectType(),
collation.GetObjectName(),
"",
))
}
return issues, nil
}

//=============NO-OP ISSUE DETECTOR ===========================

// Need to handle all the cases for which we don't have any issues detector
Expand Down Expand Up @@ -646,6 +667,8 @@ func (p *ParserIssueDetector) GetDDLDetector(obj queryparser.DDLObject) (DDLIssu
return &ViewIssueDetector{}, nil
case *queryparser.MView:
return &MViewIssueDetector{}, nil
case *queryparser.Collation:
return &CollationIssueDetector{}, nil
default:
return &NoOpIssueDetector{}, nil
}
Expand Down
29 changes: 29 additions & 0 deletions yb-voyager/src/query/queryissue/detectors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,35 @@ WHERE JSON_EXISTS(details, '$.price ? (@ > $price)' PASSING 30 AS price);`

}

func TestMergeStatementDetector(t *testing.T) {
sqls := []string{
`MERGE INTO customer_account ca
USING recent_transactions t
ON t.customer_id = ca.customer_id
WHEN MATCHED THEN
UPDATE SET balance = balance + transaction_value
WHEN NOT MATCHED THEN
INSERT (customer_id, balance)
VALUES (t.customer_id, t.transaction_value);`,

`MERGE INTO wines w
USING wine_stock_changes s
ON s.winename = w.winename
WHEN NOT MATCHED AND s.stock_delta > 0 THEN
INSERT VALUES(s.winename, s.stock_delta)
WHEN MATCHED AND w.stock + s.stock_delta > 0 THEN
UPDATE SET stock = w.stock + s.stock_delta
WHEN MATCHED THEN
DELETE
RETURNING merge_action(), w.*;`,
}

for _, sql := range sqls {
issues := getDetectorIssues(t, NewMergeStatementDetector(sql), sql)
assert.Equal(t, 1, len(issues), "Expected 1 issue for SQL: %s", sql)
assert.Equal(t, MERGE_STATEMENT, issues[0].Type, "Expected Advisory Locks issue for SQL: %s", sql)
}
}
func TestIsJsonPredicate(t *testing.T) {
sql := `SELECT js, js IS JSON "json?" FROM (VALUES ('123'), ('"abc"'), ('{"a": "b"}'), ('[1,2]'),('abc')) foo(js);`

Expand Down
13 changes: 13 additions & 0 deletions yb-voyager/src/query/queryissue/issues_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,19 @@ func NewSecurityInvokerViewIssue(objectType string, objectName string, SqlStatem
return newQueryIssue(securityInvokerViewIssue, objectType, objectName, SqlStatement, map[string]interface{}{})
}

var deterministicOptionCollationIssue = issue.Issue{
sanyamsinghal marked this conversation as resolved.
Show resolved Hide resolved
Type: DETERMINISTIC_OPTION_WITH_COLLATION,
Name: DETERMINISTIC_OPTION_WITH_COLLATION_NAME,
Impact: constants.IMPACT_LEVEL_1,
Suggestion: "This feature is not supported in YugabyteDB yet",
GH: "", // TODO
DocsLink: "", // TODO
}

func NewDeterministicOptionCollationIssue(objectType string, objectName string, SqlStatement string) QueryIssue {
return newQueryIssue(deterministicOptionCollationIssue, objectType, objectName, SqlStatement, map[string]interface{}{})
}

var foreignKeyReferencesPartitionedTableIssue = issue.Issue{
Type: FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE,
Name: FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME,
Expand Down
Loading