diff --git a/migtests/tests/analyze-schema/expected_issues.json b/migtests/tests/analyze-schema/expected_issues.json index 2830a4f59..6b44e2beb 100644 --- a/migtests/tests/analyze-schema/expected_issues.json +++ b/migtests/tests/analyze-schema/expected_issues.json @@ -534,17 +534,6 @@ "GH": "https://github.com/yugabyte/yugabyte-db/issues/10652", "MinimumVersionsFixedIn": null }, - { - "IssueType": "unsupported_features", - "ObjectType": "TABLE", - "ObjectName": "tbl_unlogged", - "Reason": "UNLOGGED tables are not supported yet.", - "SqlStatement": "CREATE UNLOGGED TABLE tbl_unlogged (id int, val text);", - "Suggestion": "Remove UNLOGGED keyword to make it work", - "GH": "https://github.com/yugabyte/yugabyte-db/issues/1129/", - "DocsLink":"https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#unlogged-table-is-not-supported", - "MinimumVersionsFixedIn": null - }, { "IssueType": "unsupported_features", "ObjectType": "VIEW", diff --git a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json index 7bac4c30e..602170979 100644 --- a/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json +++ b/migtests/tests/pg/assessment-report-test/expectedAssessmentReport.json @@ -539,17 +539,7 @@ }, { "FeatureName": "Unlogged tables", - "Objects": [ - { - "ObjectName": "public.tbl_unlogged", - "SqlStatement": "CREATE UNLOGGED TABLE public.tbl_unlogged (\n id integer,\n val text\n);" - }, - { - "ObjectName": "schema2.tbl_unlogged", - "SqlStatement": "CREATE UNLOGGED TABLE schema2.tbl_unlogged (\n id integer,\n val text\n);" - } - ], - "DocsLink": "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#unlogged-table-is-not-supported", + "Objects": [], "MinimumVersionsFixedIn": null }, { @@ -1989,7 +1979,9 @@ "SizeInBytes": 8192 } ], - "Notes": null, + "Notes": [ + "There are some Unlogged tables in the schema. They will be created as regular LOGGED tables in YugabyteDB as unlogged tables are not supported." + ], "MigrationCaveats": [ { "FeatureName": "Alter partitioned tables to add Primary Key", diff --git a/yb-voyager/cmd/assessMigrationCommand.go b/yb-voyager/cmd/assessMigrationCommand.go index 30d6cd1b1..95270d6f4 100644 --- a/yb-voyager/cmd/assessMigrationCommand.go +++ b/yb-voyager/cmd/assessMigrationCommand.go @@ -1336,7 +1336,8 @@ To manually modify the schema, please refer: https://github.com/yugabyte/yugabyte-db/issues/7850 so take a look and modify them if not supported.` + GIN_INDEXES = `There are some BITMAP indexes present in the schema that will get converted to GIN indexes, but GIN indexes are partially supported in YugabyteDB as mentioned in https://github.com/yugabyte/yugabyte-db/issues/7850 so take a look and modify them if not supported.` + UNLOGGED_TABLE_NOTE = `There are some Unlogged tables in the schema. They will be created as regular LOGGED tables in YugabyteDB as unlogged tables are not supported.` ) const FOREIGN_TABLE_NOTE = `There are some Foreign tables in the schema, but during the export schema phase, exported schema does not include the SERVER and USER MAPPING objects. Therefore, you must manually create these objects before import schema. For more information on each of them, run analyze-schema. ` @@ -1363,7 +1364,12 @@ func addNotesToAssessmentReport() { } } } + case POSTGRESQL: + if parserIssueDetector.IsUnloggedTablesIssueFiltered { + assessmentReport.Notes = append(assessmentReport.Notes, UNLOGGED_TABLE_NOTE) + } } + } func addMigrationCaveatsToAssessmentReport(unsupportedDataTypesForLiveMigration []utils.TableColumnsDataTypes, unsupportedDataTypesForLiveMigrationWithFForFB []utils.TableColumnsDataTypes) { diff --git a/yb-voyager/cmd/importData.go b/yb-voyager/cmd/importData.go index 000c4d951..3331df51b 100644 --- a/yb-voyager/cmd/importData.go +++ b/yb-voyager/cmd/importData.go @@ -16,7 +16,6 @@ limitations under the License. package cmd import ( - "context" "fmt" "io" "os" @@ -29,7 +28,6 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/fatih/color" - "github.com/jackc/pgx/v4" "github.com/samber/lo" log "github.com/sirupsen/logrus" "github.com/sourcegraph/conc/pool" @@ -1216,65 +1214,6 @@ func importBatch(batch *Batch, importBatchArgsProto *tgtdb.ImportBatchArgs) { } } -func newTargetConn() *pgx.Conn { - conn, err := pgx.Connect(context.Background(), tconf.GetConnectionUri()) - if err != nil { - utils.WaitChannel <- 1 - <-utils.WaitChannel - utils.ErrExit("connect to target db: %s", err) - } - - setTargetSchema(conn) - - if sourceDBType == ORACLE && enableOrafce { - setOrafceSearchPath(conn) - } - - return conn -} - -// TODO: Eventually get rid of this function in favour of TargetYugabyteDB.setTargetSchema(). -func setTargetSchema(conn *pgx.Conn) { - if sourceDBType == POSTGRESQL || tconf.Schema == YUGABYTEDB_DEFAULT_SCHEMA { - // For PG, schema name is already included in the object name. - // No need to set schema if importing in the default schema. - return - } - checkSchemaExistsQuery := fmt.Sprintf("SELECT count(schema_name) FROM information_schema.schemata WHERE schema_name = '%s'", tconf.Schema) - var cntSchemaName int - - if err := conn.QueryRow(context.Background(), checkSchemaExistsQuery).Scan(&cntSchemaName); err != nil { - utils.ErrExit("run query %q on target %q to check schema exists: %s", checkSchemaExistsQuery, tconf.Host, err) - } else if cntSchemaName == 0 { - utils.ErrExit("schema '%s' does not exist in target", tconf.Schema) - } - - setSchemaQuery := fmt.Sprintf("SET SCHEMA '%s'", tconf.Schema) - _, err := conn.Exec(context.Background(), setSchemaQuery) - if err != nil { - utils.ErrExit("run query %q on target %q: %s", setSchemaQuery, tconf.Host, err) - } -} - -func dropIdx(conn *pgx.Conn, idxName string) error { - dropIdxQuery := fmt.Sprintf("DROP INDEX IF EXISTS %s", idxName) - log.Infof("Dropping index: %q", dropIdxQuery) - _, err := conn.Exec(context.Background(), dropIdxQuery) - if err != nil { - return fmt.Errorf("failed to drop index %q: %w", idxName, err) - } - return nil -} - -func setOrafceSearchPath(conn *pgx.Conn) { - // append oracle schema in the search_path for orafce - updateSearchPath := `SELECT set_config('search_path', current_setting('search_path') || ', oracle', false)` - _, err := conn.Exec(context.Background(), updateSearchPath) - if err != nil { - utils.ErrExit("unable to update search_path for orafce extension: %v", err) - } -} - func getIndexName(sqlQuery string, indexName string) (string, error) { // Return the index name itself if it is aleady qualified with schema name if len(strings.Split(indexName, ".")) == 2 { @@ -1292,63 +1231,6 @@ func getIndexName(sqlQuery string, indexName string) (string, error) { return "", fmt.Errorf("could not find `ON` keyword in the CREATE INDEX statement") } -// TODO: need automation tests for this, covering cases like schema(public vs non-public) or case sensitive names -func beforeIndexCreation(sqlInfo sqlInfo, conn **pgx.Conn, objType string) error { - if !strings.Contains(strings.ToUpper(sqlInfo.stmt), "CREATE INDEX") { - return nil - } - - fullyQualifiedObjName, err := getIndexName(sqlInfo.stmt, sqlInfo.objName) - if err != nil { - return fmt.Errorf("extract qualified index name from DDL [%v]: %w", sqlInfo.stmt, err) - } - if invalidTargetIndexesCache == nil { - invalidTargetIndexesCache, err = getInvalidIndexes(conn) - if err != nil { - return fmt.Errorf("failed to fetch invalid indexes: %w", err) - } - } - - // check index valid or not - if invalidTargetIndexesCache[fullyQualifiedObjName] { - log.Infof("index %q already exists but in invalid state, dropping it", fullyQualifiedObjName) - err = dropIdx(*conn, fullyQualifiedObjName) - if err != nil { - return fmt.Errorf("drop invalid index %q: %w", fullyQualifiedObjName, err) - } - } - - // print the index name as index creation takes time and user can see the progress - color.Yellow("creating index %s ...", fullyQualifiedObjName) - return nil -} - -func getInvalidIndexes(conn **pgx.Conn) (map[string]bool, error) { - var result = make(map[string]bool) - // NOTE: this shouldn't fetch any predefined indexes of pg_catalog schema (assuming they can't be invalid) or indexes of other successful migrations - query := "SELECT indexrelid::regclass FROM pg_index WHERE indisvalid = false" - - rows, err := (*conn).Query(context.Background(), query) - if err != nil { - return nil, fmt.Errorf("querying invalid indexes: %w", err) - } - defer rows.Close() - - for rows.Next() { - var fullyQualifiedIndexName string - err := rows.Scan(&fullyQualifiedIndexName) - if err != nil { - return nil, fmt.Errorf("scanning row for invalid index name: %w", err) - } - // if schema is not provided by catalog table, then it is public schema - if !strings.Contains(fullyQualifiedIndexName, ".") { - fullyQualifiedIndexName = fmt.Sprintf("public.%s", fullyQualifiedIndexName) - } - result[fullyQualifiedIndexName] = true - } - return result, nil -} - // TODO: This function is a duplicate of the one in tgtdb/yb.go. Consolidate the two. func getTargetSchemaName(tableName string) string { parts := strings.Split(tableName, ".") diff --git a/yb-voyager/cmd/importSchemaYugabyteDB.go b/yb-voyager/cmd/importSchemaYugabyteDB.go index ecdf28446..9cb37b96d 100644 --- a/yb-voyager/cmd/importSchemaYugabyteDB.go +++ b/yb-voyager/cmd/importSchemaYugabyteDB.go @@ -23,7 +23,9 @@ import ( "time" "github.com/fatih/color" - "github.com/jackc/pgx/v4" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/samber/lo" log "github.com/sirupsen/logrus" "golang.org/x/exp/slices" @@ -34,6 +36,10 @@ import ( var deferredSqlStmts []sqlInfo var finalFailedSqlStmts []string +// The client message (NOTICE/WARNING) from psql is stored in this global variable. +// as part of the noticeHandler function for every query executed. +var notice *pgconn.Notice + func importSchemaInternal(exportDir string, importObjectList []string, skipFn func(string, string) bool) error { schemaDir := filepath.Join(exportDir, "schema") @@ -93,14 +99,13 @@ func executeSqlFile(file string, objType string, skipFn func(string, string) boo } if objType == "TABLE" { - stmt := strings.ToUpper(sqlInfo.stmt) // Check if the statement should be skipped - skip, err := shouldSkipDDL(stmt) + skip, err := shouldSkipDDL(sqlInfo.stmt) if err != nil { - return fmt.Errorf("error checking whether to skip DDL: %v", err) + return fmt.Errorf("error checking whether to skip DDL for statement [%s]: %v", sqlInfo.stmt, err) } if skip { - log.Infof("Skipping DDL: %s", stmt) + log.Infof("Skipping DDL: %s", sqlInfo.stmt) continue } } @@ -114,6 +119,13 @@ func executeSqlFile(file string, objType string, skipFn func(string, string) boo } func shouldSkipDDL(stmt string) (bool, error) { + stmt = strings.ToUpper(stmt) + + // pg_dump generate `SET client_min_messages = 'warning';`, but we want to get + // NOTICE severity as well (which is the default), hence skipping this. + if strings.Contains(stmt, "SET CLIENT_MIN_MESSAGES") { + return true, nil + } skipReplicaIdentity := strings.Contains(stmt, "ALTER TABLE") && strings.Contains(stmt, "REPLICA IDENTITY") if skipReplicaIdentity { return true, nil @@ -133,6 +145,7 @@ func shouldSkipDDL(stmt string) (bool, error) { func executeSqlStmtWithRetries(conn **pgx.Conn, sqlInfo sqlInfo, objType string) error { var err error + var stmtNotice *pgconn.Notice log.Infof("On %s run query:\n%s\n", tconf.Host, sqlInfo.formattedStmt) for retryCount := 0; retryCount <= DDL_MAX_RETRY_COUNT; retryCount++ { if retryCount > 0 { // Not the first iteration. @@ -149,9 +162,10 @@ func executeSqlStmtWithRetries(conn **pgx.Conn, sqlInfo sqlInfo, objType string) return fmt.Errorf("before index creation: %w", err) } } - _, err = (*conn).Exec(context.Background(), sqlInfo.formattedStmt) + stmtNotice, err = execStmtAndGetNotice(*conn, sqlInfo.formattedStmt) if err == nil { - utils.PrintSqlStmtIfDDL(sqlInfo.stmt, utils.GetObjectFileName(filepath.Join(exportDir, "schema"), objType)) + utils.PrintSqlStmtIfDDL(sqlInfo.stmt, utils.GetObjectFileName(filepath.Join(exportDir, "schema"), objType), + getNoticeMessage(stmtNotice)) return nil } @@ -203,7 +217,8 @@ func executeSqlStmtWithRetries(conn **pgx.Conn, sqlInfo sqlInfo, objType string) if missingRequiredSchemaObject(err) { // Do nothing for deferred case } else { - utils.PrintSqlStmtIfDDL(sqlInfo.stmt, utils.GetObjectFileName(filepath.Join(exportDir, "schema"), objType)) + utils.PrintSqlStmtIfDDL(sqlInfo.stmt, utils.GetObjectFileName(filepath.Join(exportDir, "schema"), objType), + getNoticeMessage(stmtNotice)) color.Red(fmt.Sprintf("%s\n", err.Error())) if tconf.ContinueOnError { log.Infof("appending stmt to failedSqlStmts list: %s\n", utils.GetSqlStmtToPrint(sqlInfo.stmt)) @@ -242,9 +257,14 @@ func importDeferredStatements() { beforeDeferredSqlCount := len(deferredSqlStmts) var failedSqlStmtInIthIteration []string for j := 0; j < len(deferredSqlStmts); j++ { - _, err = conn.Exec(context.Background(), deferredSqlStmts[j].formattedStmt) + var stmtNotice *pgconn.Notice + stmtNotice, err = execStmtAndGetNotice(conn, deferredSqlStmts[j].formattedStmt) if err == nil { utils.PrintAndLog("%s\n", utils.GetSqlStmtToPrint(deferredSqlStmts[j].stmt)) + noticeMsg := getNoticeMessage(stmtNotice) + if noticeMsg != "" { + utils.PrintAndLog(color.YellowString("%s\n", noticeMsg)) + } // removing successfully executed SQL deferredSqlStmts = append(deferredSqlStmts[:j], deferredSqlStmts[j+1:]...) break @@ -311,3 +331,165 @@ func applySchemaObjectFilterFlags(importObjectOrderList []string) []string { } return finalImportObjectList } + +func getInvalidIndexes(conn **pgx.Conn) (map[string]bool, error) { + var result = make(map[string]bool) + // NOTE: this shouldn't fetch any predefined indexes of pg_catalog schema (assuming they can't be invalid) or indexes of other successful migrations + query := "SELECT indexrelid::regclass FROM pg_index WHERE indisvalid = false" + + rows, err := (*conn).Query(context.Background(), query) + if err != nil { + return nil, fmt.Errorf("querying invalid indexes: %w", err) + } + defer rows.Close() + + for rows.Next() { + var fullyQualifiedIndexName string + err := rows.Scan(&fullyQualifiedIndexName) + if err != nil { + return nil, fmt.Errorf("scanning row for invalid index name: %w", err) + } + // if schema is not provided by catalog table, then it is public schema + if !strings.Contains(fullyQualifiedIndexName, ".") { + fullyQualifiedIndexName = fmt.Sprintf("public.%s", fullyQualifiedIndexName) + } + result[fullyQualifiedIndexName] = true + } + return result, nil +} + +// TODO: need automation tests for this, covering cases like schema(public vs non-public) or case sensitive names +func beforeIndexCreation(sqlInfo sqlInfo, conn **pgx.Conn, objType string) error { + if !strings.Contains(strings.ToUpper(sqlInfo.stmt), "CREATE INDEX") { + return nil + } + + fullyQualifiedObjName, err := getIndexName(sqlInfo.stmt, sqlInfo.objName) + if err != nil { + return fmt.Errorf("extract qualified index name from DDL [%v]: %w", sqlInfo.stmt, err) + } + if invalidTargetIndexesCache == nil { + invalidTargetIndexesCache, err = getInvalidIndexes(conn) + if err != nil { + return fmt.Errorf("failed to fetch invalid indexes: %w", err) + } + } + + // check index valid or not + if invalidTargetIndexesCache[fullyQualifiedObjName] { + log.Infof("index %q already exists but in invalid state, dropping it", fullyQualifiedObjName) + err = dropIdx(*conn, fullyQualifiedObjName) + if err != nil { + return fmt.Errorf("drop invalid index %q: %w", fullyQualifiedObjName, err) + } + } + + // print the index name as index creation takes time and user can see the progress + color.Yellow("creating index %s ...", fullyQualifiedObjName) + return nil +} + +func dropIdx(conn *pgx.Conn, idxName string) error { + dropIdxQuery := fmt.Sprintf("DROP INDEX IF EXISTS %s", idxName) + log.Infof("Dropping index: %q", dropIdxQuery) + _, err := conn.Exec(context.Background(), dropIdxQuery) + if err != nil { + return fmt.Errorf("failed to drop index %q: %w", idxName, err) + } + return nil +} + +func newTargetConn() *pgx.Conn { + // save notice in global variable + noticeHandler := func(conn *pgconn.PgConn, n *pgconn.Notice) { + // ALTER TABLE .. ADD PRIMARY KEY throws the following notice in YugabyteDB. + // unlogged=# ALTER TABLE ONLY public.ul ADD CONSTRAINT ul_pkey PRIMARY KEY (id); + // NOTICE: table rewrite may lead to inconsistencies + // DETAIL: Concurrent DMLs may not be reflected in the new table. + // HINT: See https://github.com/yugabyte/yugabyte-db/issues/19860. Set 'ysql_suppress_unsafe_alter_notice' yb-tserver gflag to true to suppress this notice. + + // We ignore this notice because: + // 1. This is an empty table at the time at which we are importing the schema + // and there is no concurrent DMLs + // 2. This would unnecessarily clutter the output with NOTICES for every table, + // and scare the user + noticesToIgnore := []string{ + "table rewrite may lead to inconsistencies", + } + + if n != nil { + if lo.Contains(noticesToIgnore, n.Message) { + notice = nil + return + } + } + notice = n + } + errExit := func(err error) { + if err != nil { + utils.WaitChannel <- 1 + <-utils.WaitChannel + utils.ErrExit("connect to target db: %s", err) + } + } + + conf, err := pgx.ParseConfig(tconf.GetConnectionUri()) + errExit(err) + conf.OnNotice = noticeHandler + + conn, err := pgx.ConnectConfig(context.Background(), conf) + errExit(err) + + setTargetSchema(conn) + + if sourceDBType == ORACLE && enableOrafce { + setOrafceSearchPath(conn) + } + + return conn +} + +func getNoticeMessage(n *pgconn.Notice) string { + if n == nil { + return "" + } + return fmt.Sprintf("%s: %s", n.Severity, n.Message) +} + +// TODO: Eventually get rid of this function in favour of TargetYugabyteDB.setTargetSchema(). +func setTargetSchema(conn *pgx.Conn) { + if sourceDBType == POSTGRESQL || tconf.Schema == YUGABYTEDB_DEFAULT_SCHEMA { + // For PG, schema name is already included in the object name. + // No need to set schema if importing in the default schema. + return + } + checkSchemaExistsQuery := fmt.Sprintf("SELECT count(schema_name) FROM information_schema.schemata WHERE schema_name = '%s'", tconf.Schema) + var cntSchemaName int + + if err := conn.QueryRow(context.Background(), checkSchemaExistsQuery).Scan(&cntSchemaName); err != nil { + utils.ErrExit("run query %q on target %q to check schema exists: %s", checkSchemaExistsQuery, tconf.Host, err) + } else if cntSchemaName == 0 { + utils.ErrExit("schema '%s' does not exist in target", tconf.Schema) + } + + setSchemaQuery := fmt.Sprintf("SET SCHEMA '%s'", tconf.Schema) + _, err := conn.Exec(context.Background(), setSchemaQuery) + if err != nil { + utils.ErrExit("run query %q on target %q: %s", setSchemaQuery, tconf.Host, err) + } +} + +func setOrafceSearchPath(conn *pgx.Conn) { + // append oracle schema in the search_path for orafce + updateSearchPath := `SELECT set_config('search_path', current_setting('search_path') || ', oracle', false)` + _, err := conn.Exec(context.Background(), updateSearchPath) + if err != nil { + utils.ErrExit("unable to update search_path for orafce extension: %v", err) + } +} + +func execStmtAndGetNotice(conn *pgx.Conn, stmt string) (*pgconn.Notice, error) { + notice = nil // reset notice. + _, err := conn.Exec(context.Background(), stmt) + return notice, err +} diff --git a/yb-voyager/src/query/queryissue/issues_ddl.go b/yb-voyager/src/query/queryissue/issues_ddl.go index 390cdf31d..08fe2c69c 100644 --- a/yb-voyager/src/query/queryissue/issues_ddl.go +++ b/yb-voyager/src/query/queryissue/issues_ddl.go @@ -44,6 +44,9 @@ var unloggedTableIssue = issue.Issue{ GH: "https://github.com/yugabyte/yugabyte-db/issues/1129/", Suggestion: "Remove UNLOGGED keyword to make it work", DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#unlogged-table-is-not-supported", + MinimumVersionsFixedIn: map[string]*ybversion.YBVersion{ + ybversion.SERIES_2024_2: ybversion.V2024_2_0_0, + }, } func NewUnloggedTableIssue(objectType string, objectName string, sqlStatement string) QueryIssue { diff --git a/yb-voyager/src/query/queryissue/issues_ddl_test.go b/yb-voyager/src/query/queryissue/issues_ddl_test.go index bc9beebf2..887b2759a 100644 --- a/yb-voyager/src/query/queryissue/issues_ddl_test.go +++ b/yb-voyager/src/query/queryissue/issues_ddl_test.go @@ -22,7 +22,6 @@ import ( "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" @@ -73,32 +72,6 @@ func assertErrorCorrectlyThrownForIssueForYBVersion(t *testing.T, execErr error, } } -func getConnWithNoticeHandler(noticeHandler func(*pgconn.PgConn, *pgconn.Notice)) (*pgx.Conn, error) { - ctx := context.Background() - var connStr string - var err error - if testYugabytedbConnStr != "" { - connStr = testYugabytedbConnStr - } else { - connStr, err = testYugabytedbContainer.YSQLConnectionString(ctx, "sslmode=disable") - if err != nil { - return nil, err - } - } - - conf, err := pgx.ParseConfig(connStr) - if err != nil { - return nil, err - } - conf.OnNotice = noticeHandler - conn, err := pgx.ConnectConfig(ctx, conf) - if err != nil { - return nil, err - } - - return conn, nil -} - func testXMLFunctionIssue(t *testing.T) { ctx := context.Background() conn, err := getConn() @@ -126,26 +99,14 @@ func testStoredGeneratedFunctionsIssue(t *testing.T) { } func testUnloggedTableIssue(t *testing.T) { - noticeFound := false - noticeHandler := func(conn *pgconn.PgConn, notice *pgconn.Notice) { - if notice != nil && notice.Message != "" { - assert.Equal(t, "unlogged option is currently ignored in YugabyteDB, all non-temp tables will be logged", notice.Message) - noticeFound = true - } - } ctx := context.Background() - conn, err := getConnWithNoticeHandler(noticeHandler) + conn, err := getConn() assert.NoError(t, err) defer conn.Close(context.Background()) _, err = conn.Exec(ctx, "CREATE UNLOGGED TABLE unlogged_table (a int)") - // in 2024.2, UNLOGGED no longer throws an error, just a notice - if noticeFound { - return - } else { - assert.ErrorContains(t, err, "UNLOGGED database object not supported yet") - } + assertErrorCorrectlyThrownForIssueForYBVersion(t, err, "UNLOGGED database object not supported yet", unloggedTableIssue) } func testAlterTableAddPKOnPartitionIssue(t *testing.T) { diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector.go b/yb-voyager/src/query/queryissue/parser_issue_detector.go index 2fc872014..e670ccc67 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector.go @@ -59,6 +59,10 @@ type ParserIssueDetector struct { //Boolean to check if there are any Gin indexes IsGinIndexPresentInSchema bool + + // Boolean to check if there are any unlogged tables that were filtered + // out because they are fixed as per the target db version + IsUnloggedTablesIssueFiltered bool } func NewParserIssueDetector() *ParserIssueDetector { @@ -113,6 +117,10 @@ func (p *ParserIssueDetector) getIssuesNotFixedInTargetDbVersion(issues []QueryI } if !fixed { filteredIssues = append(filteredIssues, i) + } else { + if i.Issue.Type == UNLOGGED_TABLE { + p.IsUnloggedTablesIssueFiltered = true + } } } return filteredIssues, nil diff --git a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go index e9330a09e..cbb2775b2 100644 --- a/yb-voyager/src/query/queryissue/parser_issue_detector_test.go +++ b/yb-voyager/src/query/queryissue/parser_issue_detector_test.go @@ -180,7 +180,6 @@ func TestAllIssues(t *testing.T) { stmt2: []QueryIssue{ NewPercentTypeSyntaxIssue("FUNCTION", "process_order", "orders.id%TYPE"), NewStorageParameterIssue("TABLE", "public.example", "ALTER TABLE ONLY public.example ADD CONSTRAINT example_email_key UNIQUE (email) WITH (fillfactor=70);"), - NewUnloggedTableIssue("TABLE", "tbl_unlog", "CREATE UNLOGGED TABLE tbl_unlog (id int, val text);"), NewMultiColumnGinIndexIssue("INDEX", "idx_example ON example_table", "CREATE INDEX idx_example ON example_table USING gin(name, name1);"), NewUnsupportedIndexMethodIssue("INDEX", "idx_example ON schema1.example_table", "CREATE INDEX idx_example ON schema1.example_table USING gist(name);", "gist"), NewAdvisoryLocksIssue("DML_QUERY", "", "SELECT pg_advisory_unlock(orderid);"), @@ -289,3 +288,19 @@ func TestDDLIssues(t *testing.T) { } } } + +func TestUnloggedTableIssueReportedInOlderVersion(t *testing.T) { + stmt := "CREATE UNLOGGED TABLE tbl_unlog (id int, val text);" + parserIssueDetector := NewParserIssueDetector() + + // Not reported by default + issues, err := parserIssueDetector.GetDDLIssues(stmt, ybversion.LatestStable) + fatalIfError(t, err) + assert.Equal(t, 0, len(issues)) + + // older version should report the issue + issues, err = parserIssueDetector.GetDDLIssues(stmt, ybversion.V2024_1_0_0) + fatalIfError(t, err) + assert.Equal(t, 1, len(issues)) + assert.True(t, cmp.Equal(issues[0], NewUnloggedTableIssue("TABLE", "tbl_unlog", stmt))) +} diff --git a/yb-voyager/src/utils/utils.go b/yb-voyager/src/utils/utils.go index 1016a51d6..01d21b545 100644 --- a/yb-voyager/src/utils/utils.go +++ b/yb-voyager/src/utils/utils.go @@ -32,6 +32,7 @@ import ( "syscall" "time" + "github.com/fatih/color" "github.com/google/uuid" "github.com/samber/lo" log "github.com/sirupsen/logrus" @@ -445,11 +446,15 @@ func GetSqlStmtToPrint(stmt string) string { } } -func PrintSqlStmtIfDDL(stmt string, fileName string) { +func PrintSqlStmtIfDDL(stmt string, fileName string, noticeMsg string) { setOrSelectStmt := strings.HasPrefix(strings.ToUpper(stmt), "SET ") || strings.HasPrefix(strings.ToUpper(stmt), "SELECT ") if !setOrSelectStmt { fmt.Printf("%s: %s\n", fileName, GetSqlStmtToPrint(stmt)) + if noticeMsg != "" { + fmt.Printf(color.YellowString("%s\n", noticeMsg)) + log.Infof("notice for %q: %s", GetSqlStmtToPrint(stmt), noticeMsg) + } } }