From 7a68e5eafef93be0c6b6f1d1c6be37a3f3c23a76 Mon Sep 17 00:00:00 2001 From: Priyanshi Gupta Date: Thu, 23 Jan 2025 17:28:04 +0530 Subject: [PATCH] Report non-decimal integer literals (#2217) Fixes #1982 Report the non-decimal integer literals wherever possible in Schema with some caveats listed in the code. --- .../schema/functions/function.sql | 22 +++++- .../dummy-export-dir/schema/views/view.sql | 7 ++ .../tests/analyze-schema/expected_issues.json | 22 ++++++ migtests/tests/analyze-schema/summary.json | 12 +-- .../unsupported_query_constructs.sql | 8 +- .../expectedAssessmentReport.json | 26 ++++++- .../pg_assessment_report.sql | 21 +++++- yb-voyager/cmd/assessMigrationCommand.go | 2 +- yb-voyager/src/query/queryissue/constants.go | 3 + yb-voyager/src/query/queryissue/detectors.go | 75 ++++++++++++++++++- yb-voyager/src/query/queryissue/helpers.go | 11 +++ yb-voyager/src/query/queryissue/issues_dml.go | 14 ++++ .../src/query/queryissue/issues_dml_test.go | 29 +++++++ .../query/queryissue/parser_issue_detector.go | 1 + .../queryissue/parser_issue_detector_test.go | 54 ++++++++++++- .../src/query/queryparser/helpers_protomsg.go | 8 ++ 16 files changed, 295 insertions(+), 20 deletions(-) diff --git a/migtests/tests/analyze-schema/dummy-export-dir/schema/functions/function.sql b/migtests/tests/analyze-schema/dummy-export-dir/schema/functions/function.sql index 972728848..3d98476c8 100644 --- a/migtests/tests/analyze-schema/dummy-export-dir/schema/functions/function.sql +++ b/migtests/tests/analyze-schema/dummy-export-dir/schema/functions/function.sql @@ -175,6 +175,26 @@ BEGIN END; $$ LANGUAGE plpgsql; +CREATE FUNCTION public.insert_non_decimal() RETURNS void + LANGUAGE plpgsql + AS $$ +BEGIN + -- Create a table for demonstration + CREATE TEMP TABLE non_decimal_table ( + id SERIAL, + binary_value INTEGER, + octal_value INTEGER, + hex_value INTEGER + ); + SELECT 5678901234, 0x1527D27F2, 0o52237223762, 0b101010010011111010010011111110010; + -- Insert values into the table + --not reported as parser converted these values to decimal ones while giving parseTree + INSERT INTO non_decimal_table (binary_value, octal_value, hex_value) + VALUES (0b1010, 0o012, 0xA); -- Binary (10), Octal (10), Hexadecimal (10) + RAISE NOTICE 'Row inserted with non-decimal integers.'; +END; +$$; + CREATE FUNCTION public.asterisks(n integer) RETURNS SETOF text LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE BEGIN ATOMIC @@ -187,4 +207,4 @@ CREATE FUNCTION add(int, int) RETURNS int IMMUTABLE PARALLEL SAFE BEGIN ATOMIC; CREATE FUNCTION public.asterisks1(n integer) RETURNS text LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE - RETURN repeat('*'::text, n); \ No newline at end of file + RETURN repeat('*'::text, n); diff --git a/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql b/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql index 1fc6a82cb..e16d99d18 100644 --- a/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql +++ b/migtests/tests/analyze-schema/dummy-export-dir/schema/views/view.sql @@ -49,3 +49,10 @@ SELECT jt.* FROM NESTED PATH '$.films[*]' COLUMNS ( title text FORMAT JSON PATH '$.title' OMIT QUOTES, director text PATH '$.director' KEEP QUOTES))) AS jt; + +CREATE VIEW zz AS + SELECT + 5678901234 AS DEC, + 0x1527D27F2 AS hex, + 0o52237223762 AS oct, + 0b101010010011111010010011111110010 AS bin; diff --git a/migtests/tests/analyze-schema/expected_issues.json b/migtests/tests/analyze-schema/expected_issues.json index 07d302d22..3d56fe89e 100644 --- a/migtests/tests/analyze-schema/expected_issues.json +++ b/migtests/tests/analyze-schema/expected_issues.json @@ -747,6 +747,17 @@ "DocsLink":"https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#referencing-clause-for-triggers", "MinimumVersionsFixedIn": null }, + { + "IssueType": "unsupported_plpgsql_objects", + "ObjectType": "FUNCTION", + "ObjectName": "public.insert_non_decimal", + "Reason": "Non decimal integer literals are not supported in YugabyteDB", + "SqlStatement": "SELECT 5678901234, 0x1527D27F2, 0o52237223762, 0b101010010011111010010011111110010;", + "Suggestion": "", + "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": "TRIGGER", @@ -2152,6 +2163,17 @@ "2.25": "2.25.0.0" } }, + { + "IssueType": "unsupported_features", + "ObjectType": "VIEW", + "ObjectName": "zz", + "Reason": "Non decimal integer literals are not supported in YugabyteDB", + "SqlStatement": "CREATE VIEW zz AS\n SELECT\n 5678901234 AS DEC,\n 0x1527D27F2 AS hex,\n 0o52237223762 AS oct,\n 0b101010010011111010010011111110010 AS bin;", + "Suggestion": "", + "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", diff --git a/migtests/tests/analyze-schema/summary.json b/migtests/tests/analyze-schema/summary.json index 77d7c992d..edf2d9d5c 100644 --- a/migtests/tests/analyze-schema/summary.json +++ b/migtests/tests/analyze-schema/summary.json @@ -44,9 +44,9 @@ }, { "ObjectType": "FUNCTION", - "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" + "TotalCount": 11, + "InvalidCount": 11, + "ObjectNames": "public.insert_non_decimal, 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", @@ -56,9 +56,9 @@ }, { "ObjectType": "VIEW", - "TotalCount": 7, - "InvalidCount": 7, - "ObjectNames": "public.my_films_view, v1, v2, test, public.orders_view, view_name, top_employees_view" + "TotalCount": 8, + "InvalidCount": 8, + "ObjectNames": "zz, public.my_films_view, v1, v2, test, public.orders_view, view_name, top_employees_view" }, { "ObjectType": "TRIGGER", diff --git a/migtests/tests/pg/assessment-report-test-uqc/unsupported_query_constructs.sql b/migtests/tests/pg/assessment-report-test-uqc/unsupported_query_constructs.sql index 150a24f18..e7cff9c95 100644 --- a/migtests/tests/pg/assessment-report-test-uqc/unsupported_query_constructs.sql +++ b/migtests/tests/pg/assessment-report-test-uqc/unsupported_query_constructs.sql @@ -60,6 +60,12 @@ SELECT * FROM sales.json_data WHERE array_column IS JSON ARRAY; +-- PG 15 supports the non-decimal integer literals e.g. hexadecimal, octal, binary +-- but this won't be reported as the PGSS will change the constant integers to parameters - $1, $2... +SELECT 1234, 0x4D2 as hex, 0o2322 as octal, 0b10011010010 as binary; + +SELECT 5678901234, 0x1527D27F2, 0o52237223762, 0b101010010011111010010011111110010 \gdesc + WITH w AS NOT MATERIALIZED ( SELECT * FROM sales.big_table ) @@ -76,4 +82,4 @@ 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; \ No newline at end of file +WHERE w2.key = 123; diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index fc90e83a1..e70c8de8d 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -2,7 +2,7 @@ "VoyagerVersion": "IGNORED", "TargetDBVersion": "IGNORED", "MigrationComplexity": "HIGH", - "MigrationComplexityExplanation": "Found 26 Level 2 issue(s) and 29 Level 3 issue(s), resulting in HIGH migration complexity", + "MigrationComplexityExplanation": "Found 28 Level 2 issue(s) and 29 Level 3 issue(s), resulting in HIGH migration complexity", "SchemaSummary": { "Description": "Objects that will be created on the target YugabyteDB.", "DbName": "pg_assessment_report", @@ -57,9 +57,10 @@ }, { "ObjectType": "FUNCTION", - "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" }, + "TotalCount": 17, + "InvalidCount": 10, + "ObjectNames": "public.insert_non_decimal, schema2.insert_non_decimal, 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, @@ -2931,6 +2932,23 @@ } ], "UnsupportedPlPgSqlObjects": [ + { + "FeatureName": "Non-decimal integer literal", + "Objects": [ + { + "ObjectType": "FUNCTION", + "ObjectName": "public.insert_non_decimal", + "SqlStatement": "SELECT 5678901234, 0x1527D27F2, 0o52237223762, 0b101010010011111010010011111110010;" + }, + { + "ObjectType": "FUNCTION", + "ObjectName": "schema2.insert_non_decimal", + "SqlStatement": "SELECT 5678901234, 0x1527D27F2, 0o52237223762, 0b101010010011111010010011111110010;" + } + ], + "DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features", + "MinimumVersionsFixedIn": null + }, { "FeatureName": "Large Object Functions", "Objects": [ 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 37db10002..83b446bfe 100644 --- a/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql +++ b/migtests/tests/pg/assessment-report-test/pg_assessment_report.sql @@ -478,6 +478,25 @@ CREATE UNIQUE INDEX users_unique_nulls_not_distinct_index_email NULLS NOT DISTINCT; +CREATE OR REPLACE FUNCTION insert_non_decimal() +RETURNS VOID AS $$ +BEGIN + -- Create a table for demonstration + CREATE TEMP TABLE non_decimal_table ( + id SERIAL, + binary_value INTEGER, + octal_value INTEGER, + hex_value INTEGER + ); + SELECT 5678901234, 0x1527D27F2, 0o52237223762, 0b101010010011111010010011111110010; + -- Insert values into the table + --not reported as parser converted these values to decimal ones while giving parseTree + INSERT INTO non_decimal_table (binary_value, octal_value, hex_value) + VALUES (0b1010, 0o012, 0xA); -- Binary (10), Octal (10), Hexadecimal (10) + + RAISE NOTICE 'Row inserted with non-decimal integers.'; +END; +$$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION asterisks(n int) RETURNS SETOF text @@ -490,4 +509,4 @@ END; CREATE OR REPLACE FUNCTION asterisks1(n int) RETURNS text LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE -RETURN repeat('*', n); \ No newline at end of file +RETURN repeat('*', n); diff --git a/yb-voyager/cmd/assessMigrationCommand.go b/yb-voyager/cmd/assessMigrationCommand.go index 1ed6f9475..f89a5fba7 100644 --- a/yb-voyager/cmd/assessMigrationCommand.go +++ b/yb-voyager/cmd/assessMigrationCommand.go @@ -1063,7 +1063,7 @@ func fetchUnsupportedPGFeaturesFromSchemaReport(schemaAnalysisReport utils.Schem 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)) - + unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.NON_DECIMAL_INTEGER_LITERAL_NAME, "", queryissue.NON_DECIMAL_INTEGER_LITERAL, 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 ab919765d..32ebf90c5 100644 --- a/yb-voyager/src/query/queryissue/constants.go +++ b/yb-voyager/src/query/queryissue/constants.go @@ -97,6 +97,9 @@ const ( 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" + + NON_DECIMAL_INTEGER_LITERAL = "NON_DECIMAL_INTEGER_LITERAL" + NON_DECIMAL_INTEGER_LITERAL_NAME = "Non-decimal integer literal" ) const ( diff --git a/yb-voyager/src/query/queryissue/detectors.go b/yb-voyager/src/query/queryissue/detectors.go index e4e6dbbe7..a254e356b 100644 --- a/yb-voyager/src/query/queryissue/detectors.go +++ b/yb-voyager/src/query/queryissue/detectors.go @@ -17,6 +17,7 @@ package queryissue import ( "slices" + "strings" mapset "github.com/deckarep/golang-set/v2" log "github.com/sirupsen/logrus" @@ -564,6 +565,75 @@ func (j *JsonPredicateExprDetector) GetIssues() []QueryIssue { return issues } +type NonDecimalIntegerLiteralDetector struct { + query string + detected bool +} + +func NewNonDecimalIntegerLiteralDetector(query string) *NonDecimalIntegerLiteralDetector { + return &NonDecimalIntegerLiteralDetector{ + query: query, + } +} +func (n *NonDecimalIntegerLiteralDetector) Detect(msg protoreflect.Message) error { + if queryparser.GetMsgFullName(msg) != queryparser.PG_QUERY_ACONST_NODE { + return nil + } + aConstNode, err := queryparser.ProtoAsAConstNode(msg) + if err != nil { + return err + } + /* + Caveats can't report this issue for cases like - + 1. DML having the constant change to parameters in PGSS - SELECT $1, $2 as binary; + 2. DDL having the CHECK or DEFAULT - + - pg_dump will not dump non-decimal literal, it will give out the decimal constant only + - even if in some user added DDL in schema file during analyze-schema it can't be detected as parse tree doesn't info + e.g. - + CREATE TABLE bitwise_example ( + id SERIAL PRIMARY KEY, + flags INT DEFAULT 0x0F CHECK (flags & 0x01 = 0x01) -- Hexadecimal bitwise check + ); + parseTree - create_stmt:{relation:{relname:"bitwise_example" inh:true relpersistence:"p" location:15} ... + table_elts:{column_def:{colname:"flags" type_name:{names:{string:{sval:"pg_catalog"}} names:{string:{sval:"int4"}} typemod:-1 + location:70} is_local:true constraints:{constraint:{contype:CONSTR_DEFAULT raw_expr:{a_const:{ival:{ival:15} location:82}} + location:74}} constraints:{constraint:{contype:CONSTR_CHECK initially_valid:true raw_expr:{a_expr:{kind:AEXPR_OP name:{string:{sval:"="}} + lexpr:{a_expr:{kind:AEXPR_OP name:{string:{sval:"&"}} lexpr:{column_ref:{fields:{string:{sval:"flags"}} location:94}} rexpr:{a_const:{ival:{ival:1} + location:102}} location:100}} rexpr:{a_const:{ival:{ival:1} location:109}} location:107}} .. + + So mostly be detecting this in PLPGSQL cases + */ + switch { + case aConstNode.GetFval() != nil: + /* + fval - float val representation in postgres + ival - integer val + Fval is only one which stores the non-decimal integers information if used in queries, e.g. SELECT 5678901234, 0o52237223762 as octal; + select_stmt:{target_list:{res_target:{val:{a_const:{fval:{fval:"5678901234"} location:9}} location:9}} + target_list:{res_target:{name:"octal" val:{a_const:{fval:{fval:"0o52237223762"} location:21}} location:21}} + + ival stores the decimal integers if non-decimal is not used in the query, e.g. SELECT 1, 2; + select_stmt:{target_list:{res_target:{val:{a_const:{ival:{ival:1} location:8}} location:8}} + target_list:{res_target:{val:{a_const:{ival:{ival:2} location:10}} location:10}} + */ + fval := aConstNode.GetFval().Fval + for _, literal := range nonDecimalIntegerLiterals { + if strings.HasPrefix(fval, literal) { + n.detected = true + } + } + } + return nil +} + +func (n *NonDecimalIntegerLiteralDetector) GetIssues() []QueryIssue { + var issues []QueryIssue + if n.detected { + issues = append(issues, NewNonDecimalIntegerLiteralIssue(DML_QUERY_OBJECT_TYPE, "", n.query)) + } + return issues +} + type CommonTableExpressionDetector struct { query string materializedClauseDetected bool @@ -580,8 +650,8 @@ func (c *CommonTableExpressionDetector) Detect(msg protoreflect.Message) error { 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 + 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 { @@ -601,4 +671,3 @@ func (c *CommonTableExpressionDetector) GetIssues() []QueryIssue { } return issues } - diff --git a/yb-voyager/src/query/queryissue/helpers.go b/yb-voyager/src/query/queryissue/helpers.go index d28ef9454..2168605e3 100644 --- a/yb-voyager/src/query/queryissue/helpers.go +++ b/yb-voyager/src/query/queryissue/helpers.go @@ -159,3 +159,14 @@ var catalogFunctionsReturningJsonb = mapset.NewThreadUnsafeSet([]string{ "jsonb_path_query_array_tz", "jsonb_path_query_first", "jsonb_path_query_first_tz", "jsonb_recv", "jsonb_set", "jsonb_set_lax", "jsonb_strip_nulls", "to_jsonb", "ts_headline", }...) + +var nonDecimalIntegerLiterals = []string{ + "0x", + "0X", + "0o", + "0O", + "0b", + "0B", + //https://github.com/pganalyze/pg_query_go/blob/38c866daa3fdb0a7af78741476d6b89029c19afe/parser/src_backend_utils_adt_numutils.c#L59C30-L61C76 + // the prefix "0x" could be "0X" as well so should check both +} diff --git a/yb-voyager/src/query/queryissue/issues_dml.go b/yb-voyager/src/query/queryissue/issues_dml.go index d01c36cde..758f2d82f 100644 --- a/yb-voyager/src/query/queryissue/issues_dml.go +++ b/yb-voyager/src/query/queryissue/issues_dml.go @@ -277,3 +277,17 @@ func NewMergeStatementIssue(objectType string, objectName string, sqlStatement s //MERGE STATEMENT is PG15 feature but MERGE .... RETURNING clause is PG17 feature so need to report it separately later. return newQueryIssue(mergeStatementIssue, objectType, objectName, sqlStatement, map[string]interface{}{}) } + +var nonDecimalIntegerLiteralIssue = issue.Issue{ + Type: NON_DECIMAL_INTEGER_LITERAL, + Name: NON_DECIMAL_INTEGER_LITERAL_NAME, + Impact: constants.IMPACT_LEVEL_2, + Description: "Non decimal integer literals are not supported in YugabyteDB", + Suggestion: "", + 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 NewNonDecimalIntegerLiteralIssue(objectType string, objectName string, sqlStatement string) QueryIssue { + return newQueryIssue(nonDecimalIntegerLiteralIssue, objectType, objectName, sqlStatement, map[string]interface{}{}) +} diff --git a/yb-voyager/src/query/queryissue/issues_dml_test.go b/yb-voyager/src/query/queryissue/issues_dml_test.go index 06d3d5137..7988ad0c6 100644 --- a/yb-voyager/src/query/queryissue/issues_dml_test.go +++ b/yb-voyager/src/query/queryissue/issues_dml_test.go @@ -334,6 +334,32 @@ GROUP BY department;` } +func testNonDecimalIntegerLiteralIssue(t *testing.T) { + ctx := context.Background() + conn, err := getConn() + assert.NoError(t, err) + sqls := []string{ + `CREATE VIEW zz AS + SELECT + 5678901234 as DEC, + 0x1527D27F2 as hex, + 0o52237223762 as oct, + 0b101010010011111010010011111110010 as bin;`, + `SELECT 5678901234, 0b101010010011111010010011111110010 as binary;`, + } + for _, sql := range sqls { + defer conn.Close(context.Background()) + _, err = conn.Exec(ctx, sql) + var errMsg string + switch { + case testYbVersion.Equal(ybversion.V2_25_0_0): + errMsg = `trailing junk after numeric literal at or near` + default: + errMsg = `syntax error at or near "as"` + } + assertErrorCorrectlyThrownForIssueForYBVersion(t, err, errMsg, nonDecimalIntegerLiteralIssue) + } +} func testCTEWithMaterializedIssue(t *testing.T) { sqls := map[string]string{` CREATE TABLE big_table(key text, ref text, c1 int, c2 int); @@ -420,6 +446,9 @@ func TestDMLIssuesInYBVersion(t *testing.T) { success = t.Run(fmt.Sprintf("%s-%s", "json type predicate", ybVersion), testJsonPredicateIssue) assert.True(t, success) + success = t.Run(fmt.Sprintf("%s-%s", "Non-decimal integer literal", ybVersion), testNonDecimalIntegerLiteralIssue) + assert.True(t, success) + success = t.Run(fmt.Sprintf("%s-%s", "cte with materialized cluase", ybVersion), testCTEWithMaterializedIssue) 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 de3450f20..38d7683f2 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector.go @@ -397,6 +397,7 @@ func (p *ParserIssueDetector) genericIssues(query string) ([]QueryIssue, error) NewJsonbSubscriptingDetector(query, p.jsonbColumns, p.getJsonbReturnTypeFunctions()), NewUniqueNullsNotDistinctDetector(query), NewJsonPredicateExprDetector(query), + NewNonDecimalIntegerLiteralDetector(query), NewCommonTableExpressionDetector(query), } 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 5705e1ee6..6041dc2cb 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -1041,6 +1041,54 @@ REFERENCES schema1.abc (id); } } +func TestNonDecimalIntegerLiteralsIssues(t *testing.T) { + sql1 := `SELECT 5678901234, 0x1527D27F2 as hex;` + sql2 := `SELECT 5678901234, 0o52237223762 as octal;` + sql3 := `SELECT 5678901234, 0b101010010011111010010011111110010 as binary;` + sql4 := `CREATE VIEW zz AS + SELECT + 5678901234 AS DEC, + 0x1527D27F2 AS hex, + 0o52237223762 AS oct, + 0b10101001001111101001001111111` + sql5 := `SELECT 5678901234, 0O52237223762 as octal;` // captial "0O" case + sqls := map[string]QueryIssue{ + sql1: NewNonDecimalIntegerLiteralIssue("DML_QUERY", "", sql1), + sql2: NewNonDecimalIntegerLiteralIssue("DML_QUERY", "", sql2), + sql3: NewNonDecimalIntegerLiteralIssue("DML_QUERY", "", sql3), + sql4: NewNonDecimalIntegerLiteralIssue("VIEW", "zz", sql4), + sql5: NewNonDecimalIntegerLiteralIssue("DML_QUERY", "", sql5), + } + parserIssueDetector := NewParserIssueDetector() + for sql, expectedIssue := range sqls { + issues, err := parserIssueDetector.GetAllIssues(sql, ybversion.LatestStable) + assert.NoError(t, err) + assert.Equal(t, 1, len(issues)) + cmp.Equal(issues[0], expectedIssue) + } + sqlsWithoutIssues := []string{ + `SELECT 1234, '0x4D2';`, //string constant starting with 0x + `SELECT $1, $2 as binary;`, // parameterised strings for constant data + //DEFAULT and check constraints are not reported because parse tree doesn't the info of non-decimal integer literal usage as it converts it to decimal + `CREATE TABLE bitwise_example ( + id SERIAL PRIMARY KEY, + flags INT DEFAULT 0x0F CHECK (flags & 0x01 = 0x01) -- Hexadecimal bitwise check +);`, + `CREATE TABLE bin_default(id int, bin_int int DEFAULT 0b1010010101 CHECK (bin_int<>0b1000010101));`, + /* + similarly this insert is also can't be reported as parser changes them to decimal integers while giving parseTree + insert_stmt:{relation:{relname:"non_decimal_table" inh:true relpersistence:"p" location:12} cols:{res_target:{name:"binary_value" ... + select_stmt:{select_stmt:{values_lists:{list:{items:{a_const:{ival:{ival:10} location:81}} items:{a_const:{ival:{ival:10} location:89}} items:{a_const:{ival:{ival:10} location:96}}}} + */ + `INSERT INTO non_decimal_table (binary_value, octal_value, hex_value) + VALUES (0b1010, 0o012, 0xA);`, + } + for _, sql := range sqlsWithoutIssues { + issues, err := parserIssueDetector.GetAllIssues(sql, ybversion.LatestStable) + assert.NoError(t, err) + assert.Equal(t, 0, len(issues)) + } +} func TestCTEIssues(t *testing.T) { sqls := []string{ `WITH w AS ( @@ -1061,21 +1109,21 @@ WHERE w2.key = 123;`, ) INSERT INTO products_log SELECT * FROM moved_rows;`, -`CREATE VIEW view1 AS + `CREATE VIEW view1 AS WITH data_cte AS NOT MATERIALIZED ( SELECT generate_series(1, 5) AS id, 'Name ' || generate_series(1, 5) AS name ) SELECT * FROM data_cte;`, -`CREATE VIEW view2 AS + `CREATE VIEW view2 AS WITH data_cte AS MATERIALIZED ( SELECT generate_series(1, 5) AS id, 'Name ' || generate_series(1, 5) AS name ) SELECT * FROM data_cte;`, -`CREATE VIEW view3 AS + `CREATE VIEW view3 AS WITH data_cte AS ( SELECT generate_series(1, 5) AS id, diff --git a/yb-voyager/src/query/queryparser/helpers_protomsg.go b/yb-voyager/src/query/queryparser/helpers_protomsg.go index c8155c3b8..e69a090ff 100644 --- a/yb-voyager/src/query/queryparser/helpers_protomsg.go +++ b/yb-voyager/src/query/queryparser/helpers_protomsg.go @@ -407,6 +407,14 @@ func ProtoAsSelectStmt(msg protoreflect.Message) (*pg_query.SelectStmt, error) { return selectStmtNode, nil } +func ProtoAsAConstNode(msg protoreflect.Message) (*pg_query.A_Const, error) { + aConstNode, ok := msg.Interface().(*pg_query.A_Const) + if !ok { + return nil, fmt.Errorf("failed to cast msg to %s", PG_QUERY_ACONST_NODE) + } + return aConstNode, nil +} + func ProtoAsCTENode(msg protoreflect.Message) (*pg_query.CommonTableExpr, error) { cteNode, ok := msg.Interface().(*pg_query.CommonTableExpr) if !ok {