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 CTE Materialized clause and Sql body in Function #2165

Merged
merged 17 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,18 @@ BEGIN
RAISE NOTICE 'High earner salary: %', temp_salary;
END LOOP;
END;
$$ LANGUAGE plpgsql;
$$ LANGUAGE plpgsql;

CREATE FUNCTION public.asterisks(n integer) RETURNS SETOF text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
BEGIN ATOMIC
SELECT repeat('*'::text, g.g) AS repeat
FROM generate_series(1, asterisks.n) g(g);
END;
--TODO fix our parser for this case - splitting it into two "CREATE FUNCTION ...FROM generate_series(1, asterisks.n) g(g);" "END;

CREATE FUNCTION add(int, int) RETURNS int IMMUTABLE PARALLEL SAFE BEGIN ATOMIC; SELECT $1 + $2; END;

CREATE FUNCTION public.asterisks1(n integer) RETURNS text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
RETURN repeat('*'::text, n);
33 changes: 33 additions & 0 deletions migtests/tests/analyze-schema/expected_issues.json
Original file line number Diff line number Diff line change
Expand Up @@ -2067,6 +2067,39 @@
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "FUNCTION",
"ObjectName": "public.asterisks1",
"Reason": "SQL Body for sql languages in function statement is not supported in YugabyteDB",
"SqlStatement": "CREATE FUNCTION public.asterisks1(n integer) RETURNS text\n LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE\n RETURN repeat('*'::text, n);",
"Suggestion": "No workaround available",
"GH": "https://github.com/yugabyte/yugabyte-db/issues/25575",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "FUNCTION",
"ObjectName": "public.asterisks",
"Reason": "SQL Body for sql languages in function statement is not supported in YugabyteDB",
"SqlStatement": "CREATE FUNCTION public.asterisks(n integer) RETURNS SETOF text\n LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE\n BEGIN ATOMIC\n SELECT repeat('*'::text, g.g) AS repeat\n FROM generate_series(1, asterisks.n) g(g);\nEND;",
"Suggestion": "No workaround available",
"GH": "https://github.com/yugabyte/yugabyte-db/issues/25575",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "FUNCTION",
"ObjectName": "add",
"Reason": "SQL Body for sql languages in function statement is not supported in YugabyteDB",
"SqlStatement": "CREATE FUNCTION add(int, int) RETURNS int IMMUTABLE PARALLEL SAFE BEGIN ATOMIC; SELECT $1 + $2; END;",
"Suggestion": "No workaround available",
"GH": "https://github.com/yugabyte/yugabyte-db/issues/25575",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_datatypes",
"ObjectType": "TABLE",
Expand Down
6 changes: 3 additions & 3 deletions migtests/tests/analyze-schema/summary.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
},
{
"ObjectType": "FUNCTION",
"TotalCount": 7,
"InvalidCount": 7,
"ObjectNames": "create_and_populate_tables, public.get_employeee_salary, get_employee_details, calculate_tax, log_salary_change, list_high_earners, copy_high_earners"
"TotalCount": 10,
"InvalidCount": 10,
"ObjectNames": "public.asterisks, add, public.asterisks1, create_and_populate_tables, public.get_employeee_salary, get_employee_details, calculate_tax, log_salary_change, list_high_earners, copy_high_earners"
},
{
"ObjectType": "PROCEDURE",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
},
{
"ObjectType": "TABLE",
"TotalCount": 7,
"TotalCount": 8,
"InvalidCount": 2,
"ObjectNames": "analytics.metrics, sales.orders, sales.test_json_chk, sales.events, sales.json_data, sales.customer_account, sales.recent_transactions"
"ObjectNames": "sales.big_table, analytics.metrics, sales.orders, sales.test_json_chk, sales.events, sales.json_data, sales.customer_account, sales.recent_transactions"
},
{
"ObjectType": "SEQUENCE",
Expand All @@ -53,6 +53,7 @@
"Sizing": {
"SizingRecommendation": {
"ColocatedTables": [
"sales.big_table",
"sales.orders",
"analytics.metrics",
"sales.customer_account",
Expand All @@ -61,7 +62,7 @@
"sales.json_data",
"sales.test_json_chk"
],
"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.",
"ColocatedReasoning": "Recommended instance type with 4 vCPU and 16 GiB memory could fit 8 objects (8 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 @@ -141,6 +142,20 @@
"ParentTableName": null,
"SizeInBytes": 8192
},
{
"SchemaName": "sales",
"ObjectName": "big_table",
"RowCount": 0,
"ColumnCount": 4,
"Reads": 0,
"Writes": 0,
"ReadsPerSecond": 0,
"WritesPerSecond": 0,
"IsIndex": false,
"ObjectType": "",
"ParentTableName": null,
"SizeInBytes": 0
},
{
"SchemaName": "sales",
"ObjectName": "recent_transactions",
Expand Down Expand Up @@ -289,6 +304,18 @@
"Query": "SELECT * \nFROM sales.json_data\nWHERE array_column IS JSON ARRAY",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"ConstructTypeName": "CTE with MATERIALIZE clause",
"Query": "WITH w AS NOT MATERIALIZED ( \n SELECT * FROM sales.big_table\n) \nSELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref\nWHERE w2.key = $1",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"ConstructTypeName": "CTE with MATERIALIZE clause",
"Query": "WITH w AS MATERIALIZED ( \n SELECT * FROM sales.big_table\n) \nSELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref\nWHERE w2.key = $1",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
}
],
"UnsupportedPlPgSqlObjects": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ CREATE TABLE sales.json_data (
unique_keys_column TEXT CHECK (unique_keys_column IS JSON WITH UNIQUE KEYS)
);

INSERT INTO public.json_data (
INSERT INTO sales.json_data (
id, data_column, object_column, array_column, scalar_column, unique_keys_column
) VALUES (
1, '{"key": "value"}',
Expand All @@ -144,3 +144,10 @@ INSERT INTO public.json_data (
4, '"hello"',
5, '{"uniqueKey1": "value1", "uniqueKey2": "value2"}'
);

CREATE TABLE sales.big_table (
key INT,
ref INT,
other_column_1 TEXT,
other_column_2 TEXT
);
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,21 @@ FROM sales.events;
SELECT *
FROM sales.json_data
WHERE array_column IS JSON ARRAY;

WITH w AS NOT MATERIALIZED (
SELECT * FROM sales.big_table
)
SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
WHERE w2.key = 123;

WITH w AS MATERIALIZED (
SELECT * FROM sales.big_table
)
SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
WHERE w2.key = 123;

WITH w AS (
SELECT * FROM sales.big_table
)
SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
WHERE w2.key = 123;
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@
},
{
"ObjectType": "FUNCTION",
"TotalCount": 11,
"InvalidCount": 4,
"ObjectNames": "public.manage_large_object, public.auditlogfunc, public.check_sales_region, public.prevent_update_shipped_without_date, public.process_combined_tbl, public.process_order, public.total, schema2.auditlogfunc, schema2.prevent_update_shipped_without_date, schema2.process_order, schema2.total" },
"TotalCount": 15,
"InvalidCount": 8,
"ObjectNames": "public.asterisks, schema2.asterisks, public.asterisks1, schema2.asterisks1, public.manage_large_object, public.auditlogfunc, public.check_sales_region, public.prevent_update_shipped_without_date, public.process_combined_tbl, public.process_order, public.total, schema2.auditlogfunc, schema2.prevent_update_shipped_without_date, schema2.process_order, schema2.total" },
{
"ObjectType": "AGGREGATE",
"TotalCount": 2,
Expand Down Expand Up @@ -503,6 +503,29 @@
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#deferrable-constraint-on-constraints-other-than-foreign-keys-is-not-supported",
"MinimumVersionsFixedIn": null
},
{
"FeatureName": "SQL Body in function",
"Objects": [
{
"ObjectName": "public.asterisks",
"SqlStatement": "CREATE FUNCTION public.asterisks(n integer) RETURNS SETOF text\n LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE\n BEGIN ATOMIC\n SELECT repeat('*'::text, g.g) AS repeat\n FROM generate_series(1, asterisks.n) g(g);\nEND;"
},
{
"ObjectName": "public.asterisks1",
"SqlStatement": "CREATE FUNCTION public.asterisks1(n integer) RETURNS text\n LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE\n RETURN repeat('*'::text, n);"
},
{
"ObjectName": "schema2.asterisks",
"SqlStatement": "CREATE FUNCTION schema2.asterisks(n integer) RETURNS SETOF text\n LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE\n BEGIN ATOMIC\n SELECT repeat('*'::text, g.g) AS repeat\n FROM generate_series(1, asterisks.n) g(g);\nEND;"
},
{
"ObjectName": "schema2.asterisks1",
"SqlStatement": "CREATE FUNCTION schema2.asterisks1(n integer) RETURNS text\n LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE\n RETURN repeat('*'::text, n);"
}
],
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"FeatureName": "View with check option",
"Objects": [
Expand Down
13 changes: 13 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 @@ -478,3 +478,16 @@ CREATE UNIQUE INDEX users_unique_nulls_not_distinct_index_email
NULLS NOT DISTINCT;



CREATE OR REPLACE FUNCTION asterisks(n int)
RETURNS SETOF text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
BEGIN ATOMIC
SELECT repeat('*', g) FROM generate_series (1, n) g;
END;
-- BEGIN ATOMIC syntax is not working with regex parser we have for functions TODO: fix

CREATE OR REPLACE FUNCTION asterisks1(n int)
RETURNS text
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE
RETURN repeat('*', n);
2 changes: 2 additions & 0 deletions yb-voyager/cmd/assessMigrationCommand.go
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,8 @@ func fetchUnsupportedPGFeaturesFromSchemaReport(schemaAnalysisReport utils.Schem
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))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.JSON_TYPE_PREDICATE_NAME, "", queryissue.JSON_TYPE_PREDICATE, schemaAnalysisReport, false))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.SQL_BODY_IN_FUNCTION_NAME, "", queryissue.SQL_BODY_IN_FUNCTION, schemaAnalysisReport, false,))
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.CTE_WITH_MATERIALIZED_CLAUSE_NAME, "", queryissue.CTE_WITH_MATERIALIZED_CLAUSE, schemaAnalysisReport, false))

return lo.Filter(unsupportedFeatures, func(f UnsupportedFeature, _ int) bool {
return len(f.Objects) > 0
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 @@ -87,6 +87,11 @@ const (
FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE = "FOREIGN_KEY_REFERENCED_PARTITIONED_TABLE"
FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_NAME = "Foreign key constraint references partitioned table"

CTE_WITH_MATERIALIZED_CLAUSE = "CTE_WITH_MATERIALIZED_CLAUSE"
CTE_WITH_MATERIALIZED_CLAUSE_NAME = "CTE with MATERIALIZE clause"

SQL_BODY_IN_FUNCTION = "SQL_BODY_IN_FUNCTION"
SQL_BODY_IN_FUNCTION_NAME = "SQL Body in function"
UNIQUE_NULLS_NOT_DISTINCT = "UNIQUE_NULLS_NOT_DISTINCT"
UNIQUE_NULLS_NOT_DISTINCT_NAME = "Unique Nulls Not Distinct"
)
Expand Down
39 changes: 39 additions & 0 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,3 +554,42 @@ func (j *JsonPredicateExprDetector) GetIssues() []QueryIssue {
}
return issues
}

type CommonTableExpressionDetector struct {
query string
materializedClauseDetected bool
}

func NewCommonTableExpressionDetector(query string) *CommonTableExpressionDetector {
return &CommonTableExpressionDetector{
query: query,
}
}

func (c *CommonTableExpressionDetector) Detect(msg protoreflect.Message) error {
if queryparser.GetMsgFullName(msg) != queryparser.PG_QUERY_CTE_NODE {
return nil
}
/*
with_clause:{ctes:{common_table_expr:{ctename:"cte" ctematerialized:CTEMaterializeNever
ctequery:{select_stmt:{target_list:{res_target:{val:{column_ref:{fields:{a_star:{}} location:939}} location:939}} from_clause:{range_var:{relname:"a" inh:true relpersistence:"p" location:946}} limit_option:LIMIT_OPTION_DEFAULT op:SETOP_NONE}} location:906}} location:901} op:SETOP_NONE}} stmt_location:898
*/
cteNode, err := queryparser.ProtoAsCTENode(msg)
if err != nil {
return err
}
if cteNode.Ctematerialized != queryparser.CTE_MATERIALIZED_DEFAULT {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment here explaining that both MATERIALIZED, NOT MATERIALIZED flags were not supported..

//MATERIALIZED / NOT MATERIALIZED clauses in CTE is not supported in YB
c.materializedClauseDetected = true
}
return nil
}

func (c *CommonTableExpressionDetector) GetIssues() []QueryIssue {
var issues []QueryIssue
if c.materializedClauseDetected {
issues = append(issues, NewCTEWithMaterializedIssue(DML_QUERY_OBJECT_TYPE, "", c.query))
}
return issues
}

21 changes: 21 additions & 0 deletions yb-voyager/src/query/queryissue/detectors_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,25 @@ func (v *ViewIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIssu
return issues, nil
}

// ================FUNCTION ISSUE DETECTOR ==================

type FunctionIssueDetector struct{}

func (v *FunctionIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIssue, error) {
function, ok := obj.(*queryparser.Function)
if !ok {
return nil, fmt.Errorf("invalid object type: expected View")
}
var issues []QueryIssue

if function.HasSqlBody {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you use just BEGIN to start a function block?

yes BEGIN can also start a function block

If so, are both BEGIN and BEGIN ATOMIC unsupported?

No only BEGIN ATOMIC is unsupported as its PG 15 feature

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, but will function.HasSqlBody be true even if the function has just BEGIN ? That would be a problem, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this HasSqlBody is true only if the parser has populated the sql_body field for function stmt, which is a new PG 15 feature for the BEGIN ATOMIC/RETURN expr type of function block. So, in normal BEGIN cases, this can't be true.

//https://www.postgresql.org/docs/15/sql-createfunction.html#:~:text=a%20new%20session.-,sql_body,-The%20body%20of
issues = append(issues, NewSqlBodyInFunctionIssue(function.GetObjectType(), function.GetObjectName(), ""))
}

return issues, nil
}

// ==============MVIEW ISSUE DETECTOR ======================

type MViewIssueDetector struct{}
Expand Down Expand Up @@ -667,6 +686,8 @@ func (p *ParserIssueDetector) GetDDLDetector(obj queryparser.DDLObject) (DDLIssu
return &ViewIssueDetector{}, nil
case *queryparser.MView:
return &MViewIssueDetector{}, nil
case *queryparser.Function:
return &FunctionIssueDetector{}, nil
case *queryparser.Collation:
return &CollationIssueDetector{}, nil
default:
Expand Down
14 changes: 14 additions & 0 deletions yb-voyager/src/query/queryissue/issues_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,20 @@ func NewForeignKeyReferencesPartitionedTableIssue(objectType string, objectName
return newQueryIssue(foreignKeyReferencesPartitionedTableIssue, objectType, objectName, SqlStatement, details)
}

var sqlBodyInFunctionIssue = issue.Issue{
Copy link
Collaborator

Choose a reason for hiding this comment

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

please define a Description for each issue moving forward.

Type: SQL_BODY_IN_FUNCTION,
Name: SQL_BODY_IN_FUNCTION_NAME,
Impact: constants.IMPACT_LEVEL_1,
Description: "SQL Body for sql languages in function statement is not supported in YugabyteDB",
Suggestion: "No workaround available",
GH: "https://github.com/yugabyte/yugabyte-db/issues/25575",
DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
}

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

var uniqueNullsNotDistinctIssue = issue.Issue{
Type: UNIQUE_NULLS_NOT_DISTINCT,
Name: UNIQUE_NULLS_NOT_DISTINCT_NAME,
Expand Down
Loading