Skip to content

Commit

Permalink
Report non-decimal integer literals (#2217)
Browse files Browse the repository at this point in the history
Fixes #1982
Report the non-decimal integer literals wherever possible in Schema with some caveats listed in the code.
  • Loading branch information
priyanshi-yb authored Jan 23, 2025
1 parent ae842f5 commit 7a68e5e
Show file tree
Hide file tree
Showing 16 changed files with 295 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Original file line number Diff line number Diff line change
Expand Up @@ -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;
22 changes: 22 additions & 0 deletions migtests/tests/analyze-schema/expected_issues.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
12 changes: 6 additions & 6 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": 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",
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
2 changes: 1 addition & 1 deletion yb-voyager/cmd/assessMigrationCommand.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions yb-voyager/src/query/queryissue/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
75 changes: 72 additions & 3 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package queryissue

import (
"slices"
"strings"

mapset "github.com/deckarep/golang-set/v2"
log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -601,4 +671,3 @@ func (c *CommonTableExpressionDetector) GetIssues() []QueryIssue {
}
return issues
}

11 changes: 11 additions & 0 deletions yb-voyager/src/query/queryissue/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
14 changes: 14 additions & 0 deletions yb-voyager/src/query/queryissue/issues_dml.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{})
}
Loading

0 comments on commit 7a68e5e

Please sign in to comment.