Skip to content

Commit

Permalink
Test and Update MInVersionsFixedIn for issue of ALTER PARTITIONED TAB…
Browse files Browse the repository at this point in the history
…LE ADD PRIMARY KEY (#2047)

- Added unit test
- Updated MinimumVersionsFixedIn
  • Loading branch information
makalaaneesh authored Dec 11, 2024
1 parent 01e1867 commit 7990563
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 35 deletions.
11 changes: 0 additions & 11 deletions migtests/tests/analyze-schema/expected_issues.json
Original file line number Diff line number Diff line change
Expand Up @@ -1319,17 +1319,6 @@
"GH": "https://github.com/yugabyte/yugabyte-db/issues/3944",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "migration_caveats",
"ObjectType": "TABLE",
"ObjectName": "public.range_columns_partition_test",
"Reason": "Adding primary key to a partitioned table is not supported yet.",
"SqlStatement": "ALTER TABLE ONLY public.range_columns_partition_test\n ADD CONSTRAINT range_columns_partition_test_pkey PRIMARY KEY (a, b);",
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#adding-primary-key-to-a-partitioned-table-results-in-an-error",
"Suggestion": "",
"GH": "https://github.com/yugabyte/yugabyte-db/issues/10074",
"MinimumVersionsFixedIn": null
},
{
"IssueType": "unsupported_features",
"ObjectType": "TABLE",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1960,17 +1960,7 @@
"MigrationCaveats": [
{
"FeatureName": "Alter partitioned tables to add Primary Key",
"Objects": [
{
"ObjectName": "public.sales_region",
"SqlStatement": "ALTER TABLE ONLY public.sales_region\n ADD CONSTRAINT sales_region_pkey PRIMARY KEY (id, region);"
},
{
"ObjectName": "schema2.sales_region",
"SqlStatement": "ALTER TABLE ONLY schema2.sales_region\n ADD CONSTRAINT sales_region_pkey PRIMARY KEY (id, region);"
}
],
"DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#adding-primary-key-to-a-partitioned-table-results-in-an-error",
"Objects": [],
"FeatureDescription": "After export schema, the ALTER table should be merged with CREATE table for partitioned tables as alter of partitioned tables to add primary key is not supported.",
"MinimumVersionsFixedIn": null
},
Expand Down
6 changes: 6 additions & 0 deletions yb-voyager/src/query/queryissue/issues_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"strings"

"github.com/yugabyte/yb-voyager/yb-voyager/src/issue"
"github.com/yugabyte/yb-voyager/yb-voyager/src/ybversion"
)

var generatedColumnsIssue = issue.Issue{
Expand Down Expand Up @@ -241,6 +242,11 @@ var alterTableAddPKOnPartitionIssue = issue.Issue{
TypeName: "Adding primary key to a partitioned table is not supported yet.",
DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#adding-primary-key-to-a-partitioned-table-results-in-an-error",
GH: "https://github.com/yugabyte/yugabyte-db/issues/10074",
MinimumVersionsFixedIn: map[string]*ybversion.YBVersion{
ybversion.SERIES_2024_1: ybversion.V2024_1_0_0,
ybversion.SERIES_2024_2: ybversion.V2024_2_0_0,
ybversion.SERIES_2_23: ybversion.V2_23_0_0,
},
}

func NewAlterTableAddPKOnPartiionIssue(objectType string, objectName string, SqlStatement string) QueryIssue {
Expand Down
70 changes: 57 additions & 13 deletions yb-voyager/src/query/queryissue/issues_ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,31 @@ import (
"context"
"fmt"
"os"
"strings"
"testing"

"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/stretchr/testify/assert"
"github.com/testcontainers/testcontainers-go/modules/yugabytedb"
"github.com/yugabyte/yb-voyager/yb-voyager/src/issue"
"github.com/yugabyte/yb-voyager/yb-voyager/src/ybversion"
)

var (
yugabytedbContainer *yugabytedb.Container
yugabytedbConnStr string
versions = []string{}
testYugabytedbContainer *yugabytedb.Container
testYugabytedbConnStr string
testYbVersion *ybversion.YBVersion
)

func getConn() (*pgx.Conn, error) {
ctx := context.Background()
var connStr string
var err error
if yugabytedbConnStr != "" {
connStr = yugabytedbConnStr
if testYugabytedbConnStr != "" {
connStr = testYugabytedbConnStr
} else {
connStr, err = yugabytedbContainer.YSQLConnectionString(ctx, "sslmode=disable")
connStr, err = testYugabytedbContainer.YSQLConnectionString(ctx, "sslmode=disable")
if err != nil {
return nil, err
}
Expand All @@ -53,14 +56,31 @@ func getConn() (*pgx.Conn, error) {
return conn, nil
}

func fatalIfError(t *testing.T, err error) {
if err != nil {
t.Fatalf("error: %v", err)
}
}

func assertErrorCorrectlyThrownForIssueForYBVersion(t *testing.T, execErr error, expectedError string, issue issue.Issue) {
isFixed, err := issue.IsFixedIn(testYbVersion)
fatalIfError(t, err)

if isFixed {
assert.NoError(t, execErr)
} else {
assert.ErrorContains(t, execErr, expectedError)
}
}

func getConnWithNoticeHandler(noticeHandler func(*pgconn.PgConn, *pgconn.Notice)) (*pgx.Conn, error) {
ctx := context.Background()
var connStr string
var err error
if yugabytedbConnStr != "" {
connStr = yugabytedbConnStr
if testYugabytedbConnStr != "" {
connStr = testYugabytedbConnStr
} else {
connStr, err = yugabytedbContainer.YSQLConnectionString(ctx, "sslmode=disable")
connStr, err = testYugabytedbContainer.YSQLConnectionString(ctx, "sslmode=disable")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -128,23 +148,44 @@ func testUnloggedTableIssue(t *testing.T) {

}

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

defer conn.Close(context.Background())
_, err = conn.Exec(ctx, `
CREATE TABLE orders2 (
order_id bigint NOT NULL,
order_date timestamp
) PARTITION BY RANGE (order_date);
ALTER TABLE orders2 ADD PRIMARY KEY (order_id,order_date)`)

assertErrorCorrectlyThrownForIssueForYBVersion(t, err, "changing primary key of a partitioned table is not yet implemented", alterTableAddPKOnPartitionIssue)
}

func TestDDLIssuesInYBVersion(t *testing.T) {
var err error
ybVersion := os.Getenv("YB_VERSION")
if ybVersion == "" {
panic("YB_VERSION env variable is not set. Set YB_VERSIONS=2024.1.3.0-b105 for example")
}

yugabytedbConnStr = os.Getenv("YB_CONN_STR")
if yugabytedbConnStr == "" {
ybVersionWithoutBuild := strings.Split(ybVersion, "-")[0]
testYbVersion, err = ybversion.NewYBVersion(ybVersionWithoutBuild)
fatalIfError(t, err)

testYugabytedbConnStr = os.Getenv("YB_CONN_STR")
if testYugabytedbConnStr == "" {
// spawn yugabytedb container
var err error
ctx := context.Background()
yugabytedbContainer, err = yugabytedb.Run(
testYugabytedbContainer, err = yugabytedb.Run(
ctx,
"yugabytedb/yugabyte:"+ybVersion,
)
assert.NoError(t, err)
defer yugabytedbContainer.Terminate(context.Background())
defer testYugabytedbContainer.Terminate(context.Background())
}

// run tests
Expand All @@ -158,4 +199,7 @@ func TestDDLIssuesInYBVersion(t *testing.T) {
success = t.Run(fmt.Sprintf("%s-%s", "unlogged table", ybVersion), testUnloggedTableIssue)
assert.True(t, success)

success = t.Run(fmt.Sprintf("%s-%s", "alter table add PK on partition", ybVersion), testAlterTableAddPKOnPartitionIssue)
assert.True(t, success)

}
11 changes: 11 additions & 0 deletions yb-voyager/src/ybversion/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,18 @@ const (

var LatestStable *YBVersion

var V2024_1_0_0 *YBVersion
var V2024_1_3_1 *YBVersion
var V2024_2_0_0 *YBVersion

var V2_23_0_0 *YBVersion

func init() {
var err error
V2024_1_0_0, err = NewYBVersion("2024.1.0.0")
if err != nil {
panic("could not create version 2024.1.0.0")
}
V2024_1_3_1, err = NewYBVersion("2024.1.3.1")
if err != nil {
panic("could not create version 2024.1.3.1")
Expand All @@ -41,5 +48,9 @@ func init() {
panic("could not create version 2024.2.0.0")
}

V2_23_0_0, err = NewYBVersion("2.23.0.0")
if err != nil {
panic("could not create version 2.23.0.0")
}
LatestStable = V2024_2_0_0
}

0 comments on commit 7990563

Please sign in to comment.