Skip to content

Commit

Permalink
Report unsupported database options under unsupported query constructs (
Browse files Browse the repository at this point in the history
#2270)

1. Reporting the CREATE DATABASE WITH [OPTIONS...] for the options not unsupported in >=PG12. fixes #1994
2. Added advisory-locks, system-columns tests in issues-integrations tests
3. Removed 2.18 from the YB version list in the `issues-test.yml` workflow
  • Loading branch information
priyanshi-yb authored Feb 6, 2025
1 parent dac3389 commit 223aa20
Show file tree
Hide file tree
Showing 12 changed files with 296 additions and 19 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/issue-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
strategy:
fail-fast: false
matrix:
version: [2.23.1.0-b220, 2024.1.3.1-b8, 2024.2.0.0-b145, 2.20.8.0-b53, 2.18.9.0-b17, 2.25.0.0-b489]
version: [2.23.1.0-b220, 2024.1.3.1-b8, 2024.2.0.0-b145, 2.20.8.0-b53, 2.25.0.0-b489]
env:
YB_VERSION: ${{ matrix.version }}
YB_CONN_STR: "postgres://yugabyte:yugabyte@127.0.0.1:5433/yugabyte"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"VoyagerVersion": "IGNORED",
"TargetDBVersion": "IGNORED",
"MigrationComplexity": "MEDIUM",
"MigrationComplexityExplanation": "Found 0 Level 1 issue(s), 19 Level 2 issue(s) and 1 Level 3 issue(s), resulting in MEDIUM migration complexity",
"MigrationComplexityExplanation": "Found 0 Level 1 issue(s), 20 Level 2 issue(s) and 1 Level 3 issue(s), resulting in MEDIUM migration complexity",
"SchemaSummary": {
"Description": "Objects that will be created on the target YugabyteDB.",
"DbName": "pg_assessment_report_uqc",
Expand Down Expand Up @@ -183,6 +183,19 @@
"2.25": "2.25.0.0"
}
},
{
"Category": "unsupported_query_constructs",
"CategoryDescription": "",
"Type": "DATABASE_OPTIONS_PG15",
"Name": "Database options",
"Description": "Database options (strategy) introduced in PostgreSQL 12 and later are not supported yet in YugabyteDB.",
"Impact": "LEVEL_2",
"ObjectType": "",
"ObjectName": "",
"SqlStatement": "CREATE DATABASE strategy_example\n WITH STRATEGY = 'wal_log'",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"Category": "unsupported_features",
"CategoryDescription": "Features of the source database that are not supported on the target YugabyteDB.",
Expand Down Expand Up @@ -691,6 +704,12 @@
"2.25": "2.25.0.0"
}
},
{
"ConstructTypeName": "Database options",
"Query": "CREATE DATABASE strategy_example\n WITH STRATEGY = 'wal_log'",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#postgresql-12-and-later-features",
"MinimumVersionsFixedIn": null
},
{
"ConstructTypeName": "Jsonb Subscripting",
"Query": "SELECT ($1 :: jsonb)[$2][$3] as b",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,9 @@ WITH w AS (
SELECT * FROM w AS w1 JOIN w AS w2 ON w1.key = w2.ref
WHERE w2.key = 123;

LISTEN my_table_changes;
CREATE DATABASE strategy_example
WITH STRATEGY = 'wal_log';

DROP DATABASE strategy_example;

LISTEN my_table_changes;
4 changes: 4 additions & 0 deletions yb-voyager/src/query/queryissue/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ const (
LISTEN_NOTIFY_NAME = "Events Listen / Notify"
NON_DECIMAL_INTEGER_LITERAL = "NON_DECIMAL_INTEGER_LITERAL"
NON_DECIMAL_INTEGER_LITERAL_NAME = "Non-decimal integer literal"

DATABASE_OPTIONS_PG15 = "DATABASE_OPTIONS_PG15"
DATABASE_OPTIONS_PG17 = "DATABASE_OPTIONS_PG17"
)

const (
Expand Down Expand Up @@ -176,6 +179,7 @@ const (
NON_DETERMINISTIC_COLLATION_ISSUE_DESCRIPTION = "Non-Deterministic collations are not yet supported in YugabyteDB."
FOREIGN_KEY_REFERENCES_PARTITIONED_TABLE_ISSUE_DESCRIPTION = "Foreign key references to partitioned table are not yet supported in YugabyteDB."
UNIQUE_NULLS_NOT_DISTINCT_ISSUE_DESCRIPTION = "Unique constraint on columns with NULL values is not yet supported in YugabyteDB."
DATABASE_OPTIONS_DESCRIPTION = "Database options (%s) introduced in PostgreSQL 12 and later are not supported yet in YugabyteDB."
)

// Object types
Expand Down
50 changes: 50 additions & 0 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,56 @@ func (c *CommonTableExpressionDetector) GetIssues() []QueryIssue {
return issues
}

type DatabaseOptionsDetector struct {
query string
dbName string
pg15OptionsDetected mapset.Set[string]
pg17OptionsDetected mapset.Set[string]
}

func NewDatabaseOptionsDetector(query string) *DatabaseOptionsDetector {
return &DatabaseOptionsDetector{
query: query,
pg15OptionsDetected: mapset.NewThreadUnsafeSet[string](),
pg17OptionsDetected: mapset.NewThreadUnsafeSet[string](),
}
}

func (d *DatabaseOptionsDetector) Detect(msg protoreflect.Message) error {
if queryparser.GetMsgFullName(msg) != queryparser.PG_QUERY_CREATEDB_STMT_NODE {
return nil
}
/*
stmts:{stmt:{createdb_stmt:{dbname:"test" options:{def_elem:{defname:"oid" arg:{integer:{ival:121231}} defaction:DEFELEM_UNSPEC
location:22}}}} stmt_len:32} stmts:{stmt:{listen_stmt:{conditionname:"my_table_changes"}} stmt_location:33 stmt_len:25}
*/
d.dbName = queryparser.GetStringField(msg, "dbname")
defNames, err := queryparser.TraverseAndExtractDefNamesFromDefElem(msg)
if err != nil {
return err
}
for defName, _ := range defNames {
if unsupportedDatabaseOptionsFromPG15.ContainsOne(defName) {
d.pg15OptionsDetected.Add(defName)
}
if unsupportedDatabaseOptionsFromPG17.ContainsOne(defName) {
d.pg17OptionsDetected.Add(defName)
}
}
return nil
}

func (d *DatabaseOptionsDetector) GetIssues() []QueryIssue {
var issues []QueryIssue
if d.pg15OptionsDetected.Cardinality() > 0 {
issues = append(issues, NewDatabaseOptionsPG15Issue("DATABASE", d.dbName, d.query, d.pg15OptionsDetected.ToSlice()))
}
if d.pg17OptionsDetected.Cardinality() > 0 {
issues = append(issues, NewDatabaseOptionsPG17Issue("DATABASE", d.dbName, d.query, d.pg17OptionsDetected.ToSlice()))
}
return issues
}

type ListenNotifyIssueDetector struct {
query string
detected bool
Expand Down
8 changes: 8 additions & 0 deletions yb-voyager/src/query/queryissue/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,11 @@ var nonDecimalIntegerLiterals = []string{
//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
}

var unsupportedDatabaseOptionsFromPG15 = mapset.NewThreadUnsafeSet([]string{
"icu_locale", "locale_provider", "locale", "strategy", "collation_version", "oid",
}...)

var unsupportedDatabaseOptionsFromPG17 = mapset.NewThreadUnsafeSet([]string{
"builtin_locale", "icu_rules",
}...)
45 changes: 44 additions & 1 deletion yb-voyager/src/query/queryissue/issues_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package queryissue

import (
"fmt"
"sort"
"strings"

"github.com/yugabyte/yb-voyager/yb-voyager/src/constants"
Expand Down Expand Up @@ -556,7 +557,7 @@ var deterministicOptionCollationIssue = issue.Issue{
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: map[string]*ybversion.YBVersion{
ybversion.SERIES_2_25: ybversion.V2_25_0_0,
ybversion.SERIES_2_25: ybversion.V2_25_0_0,
},
}

Expand Down Expand Up @@ -631,3 +632,45 @@ var uniqueNullsNotDistinctIssue = issue.Issue{
func NewUniqueNullsNotDistinctIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(uniqueNullsNotDistinctIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
}

/*
Database options works on 2.25 but not marking it as supported in 2.25 for now but as per this ticket
https://github.com/yugabyte/yugabyte-db/issues/25541, DB will be blocking this support so its not supported technically
*/
var databaseOptionsPG15Issue = issue.Issue{
Type: DATABASE_OPTIONS_PG15,
Name: "Database options",
Impact: constants.IMPACT_LEVEL_2,
Description: DATABASE_OPTIONS_DESCRIPTION,
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: map[string]*ybversion.YBVersion{
// ybversion.SERIES_2_25: ybversion.V2_25_0_0,
// },// Not marking it as supported for 2.25 as it seems these might be actually not supported at least the locale and collation ones https://github.com/yugabyte/yugabyte-db/issues/25541
}

func NewDatabaseOptionsPG15Issue(objectType string, objectName string, sqlStatement string, options []string) QueryIssue {
sort.Strings(options)
issue := databaseOptionsPG15Issue
issue.Description = fmt.Sprintf(issue.Description, strings.Join(options, ", "))
return newQueryIssue(issue, objectType, objectName, sqlStatement, map[string]interface{}{})
}

var databaseOptionsPG17Issue = issue.Issue{
Type: DATABASE_OPTIONS_PG17,
Name: "Database options",
Impact: constants.IMPACT_LEVEL_2,
Description: DATABASE_OPTIONS_DESCRIPTION,
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 NewDatabaseOptionsPG17Issue(objectType string, objectName string, sqlStatement string, options []string) QueryIssue {
sort.Strings(options)
issue := databaseOptionsPG17Issue
issue.Description = fmt.Sprintf(issue.Description, strings.Join(options, ", "))
return newQueryIssue(issue, objectType, objectName, sqlStatement, map[string]interface{}{})
}
66 changes: 54 additions & 12 deletions yb-voyager/src/query/queryissue/issues_ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,6 @@ func assertErrorCorrectlyThrownForIssueForYBVersion(t *testing.T, execErr error,
}
}

func testXMLFunctionIssue(t *testing.T) {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, "SELECT xmlconcat('<abc/>', '<bar>foo</bar>')")
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, "unsupported XML feature", xmlFunctionsIssue)
}

func testStoredGeneratedFunctionsIssue(t *testing.T) {
ctx := context.Background()
conn, err := getConn()
Expand Down Expand Up @@ -396,6 +386,56 @@ EXECUTE FUNCTION public.check_sales_region();`)
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, `"sales_region" is a partitioned table`, beforeRowTriggerOnPartitionTableIssue)
}

func testDatabaseOptions(t *testing.T) {
sqlsforPG15 := []string{
` CREATE DATABASE locale_example
WITH LOCALE = 'en_US.UTF-8'
TEMPLATE = template0;`,
`CREATE DATABASE locale_provider_example
WITH ICU_LOCALE = 'en_US'
LOCALE_PROVIDER = 'icu'
TEMPLATE = template0;`,
`CREATE DATABASE oid_example
WITH OID = 123456;`,
`CREATE DATABASE collation_version_example
WITH COLLATION_VERSION = '153.128';`,
`CREATE DATABASE strategy_example
WITH STRATEGY = 'wal_log';`,
}
sqlsForPG17 := []string{
`CREATE DATABASE icu_rules_example
WITH ICU_RULES = '&a < b < c';`,
`CREATE DATABASE builtin_locale_example
WITH BUILTIN_LOCALE = 'C';`,
}

for _, sql := range sqlsforPG15 {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, sql)
switch {
case testYbVersion.ReleaseType() == ybversion.V2_25_0_0.ReleaseType() && testYbVersion.GreaterThanOrEqual(ybversion.V2_25_0_0):
assert.NoError(t, err)
assertErrorCorrectlyThrownForIssueForYBVersion(t, fmt.Errorf(""), "", databaseOptionsPG15Issue)
default:
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, `not recognized`, databaseOptionsPG15Issue)
}
}
for _, sql := range sqlsForPG17 {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, sql)
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, `not recognized`, databaseOptionsPG17Issue)
}

}

func testNonDeterministicCollationIssue(t *testing.T) {
ctx := context.Background()
conn, err := getConn()
Expand Down Expand Up @@ -488,8 +528,6 @@ func TestDDLIssuesInYBVersion(t *testing.T) {

// run tests
var success bool
success = t.Run(fmt.Sprintf("%s-%s", "xml functions", ybVersion), testXMLFunctionIssue)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "stored generated functions", ybVersion), testStoredGeneratedFunctionsIssue)
assert.True(t, success)
Expand Down Expand Up @@ -538,4 +576,8 @@ func TestDDLIssuesInYBVersion(t *testing.T) {

success = t.Run(fmt.Sprintf("%s-%s", "before row triggers on partitioned table", ybVersion), testBeforeRowTriggerOnPartitionedTable)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "database options", ybVersion), testDatabaseOptions)
assert.True(t, success)

}
49 changes: 48 additions & 1 deletion yb-voyager/src/query/queryissue/issues_dml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,43 @@ import (
testutils "github.com/yugabyte/yb-voyager/yb-voyager/test/utils"
)

func testXMLFunctionIssue(t *testing.T) {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, "SELECT xmlconcat('<abc/>', '<bar>foo</bar>')")
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, "unsupported XML feature", xmlFunctionsIssue)
}

func testAdvisoryLocks(t *testing.T) {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, "SELECT pg_advisory_unlock_shared(100);")
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, "advisory locks are not yet implemented", advisoryLocksIssue)
}

func testSystemColumns(t *testing.T) {
ctx := context.Background()
conn, err := getConn()
assert.NoError(t, err)

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, `
CREATE TABLE system_col_test(id int, val text);
`)
assert.NoError(t, err)
for _, col := range unsupportedSysCols.ToSlice() {
query := fmt.Sprintf("select %s from system_col_test", col)
_, err = conn.Exec(ctx, query)
assertErrorCorrectlyThrownForIssueForYBVersion(t, err, fmt.Sprintf(`System column "%s" is not supported yet`, col), systemColumnsIssue)
}
}

func testLOFunctionsIssue(t *testing.T) {
ctx := context.Background()
conn, err := getConn()
Expand Down Expand Up @@ -440,7 +477,17 @@ func TestDMLIssuesInYBVersion(t *testing.T) {
}

// run tests
success := t.Run(fmt.Sprintf("%s-%s", "lo functions", ybVersion), testLOFunctionsIssue)

success := t.Run(fmt.Sprintf("%s-%s", "xml functions", ybVersion), testXMLFunctionIssue)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "advisory locks", ybVersion), testAdvisoryLocks)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "system columns", ybVersion), testSystemColumns)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "lo functions", ybVersion), testLOFunctionsIssue)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "regex functions", ybVersion), testRegexFunctionsIssue)
Expand Down
1 change: 1 addition & 0 deletions yb-voyager/src/query/queryissue/parser_issue_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ func (p *ParserIssueDetector) genericIssues(query string) ([]QueryIssue, error)
NewJsonPredicateExprDetector(query),
NewNonDecimalIntegerLiteralDetector(query),
NewCommonTableExpressionDetector(query),
NewDatabaseOptionsDetector(query),
NewListenNotifyIssueDetector(query),
}

Expand Down
Loading

0 comments on commit 223aa20

Please sign in to comment.