-
Notifications
You must be signed in to change notification settings - Fork 12
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
Report unsupported database options under unsupported query constructs #2270
Changes from all commits
de1689e
28be12a
acf0ee1
2471cd4
e9a0232
c0372f2
807a1db
2f04509
69a8b58
5e23800
a9cae42
214babe
548343f
7b5da43
28eee46
9d58b8c
23d1fd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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() | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. assertNoError, is enough, right? do we need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this function |
||
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() | ||
|
@@ -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) | ||
|
@@ -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) | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, why do we need this switch case? If it's 2.25, err will be empty, and
assertErrorCorrectlyThrownForIssueForYBVersion
will take care of it automatically, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need the switch case right, because firstly we are not marking this as fixed in 2.25 but the CREATE DATABASE works without error in the
2.25
so, we need to run this funcassertErrorCorrectlyThrownForIssueForYBVersion
with different expected and actual errors in different casesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add a comment here as well to indicate that it is not actually supported in 2.25, but does not throw any error as such