Add primary key recommendation for tables with UNIQUE NOT NULL columns (#2985)

Author: ShivanshGahlot

yb-voyager/cmd/analyzeSchema.go

@@ -1152,8 +1152,12 @@ func checkerMisc(detectPerfOptimizationIssues bool) {
 	if detectPerfOptimizationIssues {
 		fkIssues := parserIssueDetector.DetectMissingForeignKeyIndexes()
 
+		// Detect PK recommendation when UNIQUE+NOT NULL exists but PK is missing
+		pkRecs := parserIssueDetector.DetectPrimaryKeyRecommendations()
+
 		// Convert QueryIssues to AnalyzeSchemaIssues and add to report
-		for _, issue := range fkIssues {
+		issues := append(fkIssues, pkRecs...)
+		for _, issue := range issues {
 			analyzeIssue := convertIssueInstanceToAnalyzeIssue(issue, "", false, true)
 			schemaAnalysisReport.Issues = append(schemaAnalysisReport.Issues, analyzeIssue)
 		}

yb-voyager/cmd/assessMigrationCommand.go

@@ -1044,6 +1044,7 @@ func fetchUnsupportedPGFeaturesFromSchemaReport(schemaAnalysisReport utils.Schem
 	unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.HOTSPOTS_ON_TIMESTAMP_PK_UK_ISSUE, "", queryissue.HOTSPOTS_ON_TIMESTAMP_PK_UK, schemaAnalysisReport, false))
 	unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.FOREIGN_KEY_DATATYPE_MISMATCH_ISSUE_NAME, "", queryissue.FOREIGN_KEY_DATATYPE_MISMATCH, schemaAnalysisReport, false))
 	unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.MISSING_FOREIGN_KEY_INDEX_ISSUE_NAME, "", queryissue.MISSING_FOREIGN_KEY_INDEX, schemaAnalysisReport, false))
+	unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL_ISSUE_NAME, "", queryissue.MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL, schemaAnalysisReport, false))
 
 	return lo.Filter(unsupportedFeatures, func(f UnsupportedFeature, _ int) bool {
 		return len(f.Objects) > 0

yb-voyager/cmd/callhome_payload_builder.go

@@ -258,6 +258,26 @@ func anonymizeIssueDetailsForCallhome(details map[string]interface{}) map[string
 			} else {
 				anonymizedDetails[key] = value
 			}
+		} else if key == "PrimaryKeyColumnOptions" {
+			if pkOptions, ok := value.([][]string); ok {
+				var anonymizedOptions [][]string
+				for _, option := range pkOptions {
+					var anonymizedOption []string
+					for _, columnName := range option {
+						anonymizedColumn, err := anonymizer.AnonymizeQualifiedColumnName(columnName)
+						if err != nil {
+							log.Warnf("failed to anonymize PK column name %s: %v", columnName, err)
+							anonymizedOption = append(anonymizedOption, "column_xxx")
+						} else {
+							anonymizedOption = append(anonymizedOption, anonymizedColumn)
+						}
+					}
+					anonymizedOptions = append(anonymizedOptions, anonymizedOption)
+				}
+				anonymizedDetails[key] = anonymizedOptions
+			} else {
+				anonymizedDetails[key] = value
+			}
 		} else {
 			// Non-sensitive keys are kept as-is
 			anonymizedDetails[key] = value

yb-voyager/src/query/queryissue/constants.go

@@ -473,6 +473,11 @@ Note: If the table is created as colocated, this hotspot concern can safely be i
 
 	MISSING_FOREIGN_KEY_INDEX_ISSUE_NAME  = "Missing index on foreign key columns"
 	MISSING_FOREIGN_KEY_INDEX_DESCRIPTION = "Foreign key columns do not have a proper index. The index must include all foreign key columns as leading columns (either in exact order, any permutation, or as a prefix of a composite index). This can cause performance issues during DML operations on the referenced table."
+
+	// Recommend PK when UNIQUE + all NOT NULL but no PK exists
+	MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL             = "MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL"
+	MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL_ISSUE_NAME  = "Missing primary key for table when unique and not null columns exist"
+	MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL_DESCRIPTION = "YugabyteDB's table storage is index-oriented. So, a table without a primary key is not viable in YugabyteDB. If you don't specify a primary key, YugabyteDB will use the internal ybrowid column as PRIMARY KEY and the table will be sharded on ybrowid HASH. However, if there are candidate primary keys (unique + NOT NULL), it is recommended to define them as a Primary Key to avoid secondary index structures."
 )
 
 // Object types

yb-voyager/src/query/queryissue/detectors_ddl.go

@@ -129,7 +129,7 @@ func (d *TableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIss
 					obj.GetObjectType(),
 					table.GetObjectName(),
 					c.Columns,
-					d.columnMetadata,
+					d.tablesMetadata,
 					&issues,
 				)
 
@@ -144,7 +144,7 @@ func (d *TableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIss
 				))
 			}
 
-			if c.ConstraintType == queryparser.FOREIGN_CONSTR_TYPE && d.partitionedTablesMap[c.ReferencedTable] {
+			if c.ConstraintType == queryparser.FOREIGN_CONSTR_TYPE && d.getPartitionedTablesMap()[c.ReferencedTable] {
 				issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue(
 					TABLE_OBJECT_TYPE,
 					table.GetObjectName(),
@@ -285,9 +285,13 @@ func (d *TableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIss
 	return issues, nil
 }
 
-func detectForeignKeyDatatypeMismatch(objectType string, objectName string, columnList []string, columnMetadata map[string]map[string]*ColumnMetadata, issues *[]QueryIssue) {
+func detectForeignKeyDatatypeMismatch(objectType string, objectName string, columnList []string, tablesMetadata map[string]*TableMetadata, issues *[]QueryIssue) {
 	for _, col := range columnList {
-		colMetadata, ok := columnMetadata[objectName][col]
+		tm, ok := tablesMetadata[objectName]
+		if !ok {
+			continue
+		}
+		colMetadata, ok := tm.Columns[col]
 		if !ok || !colMetadata.IsForeignKey {
 			continue
 		}
@@ -1038,7 +1042,7 @@ func (aid *AlterTableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]Q
 		}
 
 		if alter.ConstraintType == queryparser.FOREIGN_CONSTR_TYPE &&
-			aid.partitionedTablesMap[alter.ConstraintReferencedTable] {
+			aid.getPartitionedTablesMap()[alter.ConstraintReferencedTable] {
 			//FK constraint references partitioned table
 			issues = append(issues, NewForeignKeyReferencesPartitionedTableIssue(
 				TABLE_OBJECT_TYPE,
@@ -1054,14 +1058,14 @@ func (aid *AlterTableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]Q
 				obj.GetObjectType(),
 				alter.GetObjectName(),
 				alter.ConstraintColumns,
-				aid.columnMetadata,
+				aid.tablesMetadata,
 				&issues,
 			)
 
 		}
 
 		if alter.ConstraintType == queryparser.PRIMARY_CONSTR_TYPE &&
-			aid.partitionedTablesMap[alter.GetObjectName()] {
+			aid.getPartitionedTablesMap()[alter.GetObjectName()] {
 			issues = append(issues, NewAlterTableAddPKOnPartiionIssue(
 				obj.GetObjectType(),
 				alter.GetObjectName(),
@@ -1180,7 +1184,7 @@ func (tid *TriggerIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]Quer
 		))
 	}
 
-	if trigger.IsBeforeRowTrigger() && tid.partitionedTablesMap[trigger.GetTableName()] {
+	if trigger.IsBeforeRowTrigger() && tid.getPartitionedTablesMap()[trigger.GetTableName()] {
 		issues = append(issues, NewBeforeRowOnPartitionTableIssue(
 			obj.GetObjectType(),
 			trigger.GetObjectName(),

yb-voyager/src/query/queryissue/issues_perf.go

@@ -236,3 +236,20 @@ func NewMissingForeignKeyIndexIssue(objectType string, objectName string, sqlSta
 
 	return newQueryIssue(issue, objectType, objectName, sqlStatement, details)
 }
+
+// NewMissingPrimaryKeyWhenUniqueNotNullIssue returns a recommendation to add a PK when a UNIQUE constraint's columns are all NOT NULL
+var missingPrimaryKeyWhenUniqueNotNullIssue = issue.Issue{
+	Name:        MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL_ISSUE_NAME,
+	Type:        MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL,
+	Impact:      constants.IMPACT_LEVEL_1,
+	Description: MISSING_PRIMARY_KEY_WHEN_UNIQUE_NOT_NULL_DESCRIPTION,
+	DocsLink:    "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#missing-primary-key-when-unique-not-null",
+}
+
+func NewMissingPrimaryKeyWhenUniqueNotNullIssue(objectType string, objectName string, options [][]string) QueryIssue {
+	issue := missingPrimaryKeyWhenUniqueNotNullIssue
+	details := map[string]interface{}{
+		"PrimaryKeyColumnOptions": options,
+	}
+	return newQueryIssue(issue, objectType, objectName, "", details)
+}