Report non-decimal integer literals (#2217)

priyanshi-yb · web-flow · commit 7a68e5eafef9 · 2025-01-23T17:28:04.000+05:30
Fixes #1982 Report the non-decimal integer literals wherever possible in Schema with some caveats listed in the code.
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
@@ -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);
+    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
@@ -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
@@ -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
@@ -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
@@ -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;
+WHERE w2.key = 123;
diff --git 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
@@ -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);
+RETURN repeat('*', n);
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector.go b/yb-voyager/src/query/queryissue/parser_issue_detector.go
diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go
diff --git a/yb-voyager/src/query/queryparser/helpers_protomsg.go b/yb-voyager/src/query/queryparser/helpers_protomsg.go

Original file line number	Diff line number	Diff line change
`@@ -97,6 +97,9 @@ const (`
`97`	`97`	`SQL_BODY_IN_FUNCTION_NAME = "SQL Body in function"`
`98`	`98`	`UNIQUE_NULLS_NOT_DISTINCT = "UNIQUE_NULLS_NOT_DISTINCT"`
`99`	`99`	`UNIQUE_NULLS_NOT_DISTINCT_NAME = "Unique Nulls Not Distinct"`
	`100`	`+`
	`101`	`+ NON_DECIMAL_INTEGER_LITERAL = "NON_DECIMAL_INTEGER_LITERAL"`
	`102`	`+ NON_DECIMAL_INTEGER_LITERAL_NAME = "Non-decimal integer literal"`
`100`	`103`	`)`
`101`	`104`
`102`	`105`	`const (`