-
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
Separate query issues for unsupported datatypes and PK/UK on complex datatypes #2361
base: main
Are you sure you want to change the base?
Conversation
…ssues in analyze schema
…ration using queryissues in analyze schema
…ration during FF and FB using queryissues in analyze schema
…ind Migration Caveats issues
1921abc
to
44b4ef6
Compare
queryissue.POINT_DATATYPE, | ||
queryissue.LINE_DATATYPE, | ||
queryissue.LSEG_DATATYPE, | ||
queryissue.BOX_DATATYPE, | ||
queryissue.PATH_DATATYPE, | ||
queryissue.POLYGON_DATATYPE, | ||
queryissue.CIRCLE_DATATYPE, |
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.
lets have a list of these in queryissue like unsupportedDatatypeIssuesInLiveMigration
and unsupportedDatatypeIssuesInLiveMigrationWithFFOrFB
so that we these list in a single place
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.
Done
yb-voyager/cmd/analyzeSchema.go
Outdated
queryissue.PK_UK_ON_TXID_SNAPSHOT_DATATYPE, | ||
queryissue.PK_UK_ON_PGLSN_DATATYPE, | ||
queryissue.PK_UK_ON_ARRAY_DATATYPE, | ||
queryissue.PK_UK_ON_USER_DEFINED_DATATYPE, |
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.
Similarly for these have a list pkOrUkOnComplexDatatypesIssues
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.
Done
log.Infof("fetching unsupported features for PK/UK on complex datatypes...") | ||
unsupportedFeatures := make([]UnsupportedFeature, 0) | ||
|
||
unsupportedFeatures = append(unsupportedFeatures, getUnsupportedFeaturesFromSchemaAnalysisReport(queryissue.PK_UK_ON_CITEXT_DATATYPE_ISSUE_NAME, "", queryissue.PK_UK_ON_CITEXT_DATATYPE, schemaAnalysisReport, false)) |
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.
once you have that list, you can use that list here directly to loop over and reduce the number of lines here.
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.
Done
@@ -222,24 +225,299 @@ func (d *TableIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIss | |||
return issues, nil | |||
} | |||
|
|||
func reportPKOrUniqueConstraintOnUnsupportedDatatypesIssue(objType string, objName string, typeName string, constraintName string, issues *[]QueryIssue) { | |||
switch typeName { |
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.
can you see if we can merge this function with already existing reportIndexOnComplexDatatypes
function as the types are same in both the cases?
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.
Done
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.
LGTM with some comments.
@@ -410,183 +612,473 @@ func (d *IndexIssueDetector) DetectIssues(obj queryparser.DDLObject) ([]QueryIss | |||
return issues, nil | |||
} | |||
|
|||
func reportIndexOnComplexDatatypesIssue(objType string, objName string, typeName string) QueryIssue { | |||
func reportIndexOnComplexDatatypesIssue(objType string, objName string, typeName string, isPkorUk bool, constraintName string) QueryIssue { |
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.
Change function name to reportIndexOrConstraintIssuesOnComplexDatatypes
@@ -623,18 +623,71 @@ var MigrationCaveatsIssues = []string{ | |||
queryissue.ALTER_TABLE_ADD_PK_ON_PARTITIONED_TABLE, | |||
queryissue.FOREIGN_TABLE, | |||
queryissue.POLICY_WITH_ROLES, | |||
queryissue.UNSUPPORTED_DATATYPE_LIVE_MIGRATION, | |||
queryissue.UNSUPPORTED_DATATYPE_LIVE_MIGRATION_WITH_FF_FB, | |||
} |
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.
Can you put this migrationCaveatsIssues
inside the convertIssueInstanceToAnalyzeIssue
, to avoid any usage of it outside that function as its not complete now.
queryissue.UNSUPPORTED_DATATYPE_LIVE_MIGRATION_WITH_FF_FB, | ||
} | ||
|
||
var UnsupportedDatatypesInLiveMigrationIssues = []string{ |
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.
put these lists in the queryissue
package.
if !isPkorUk { | ||
queryIssue = NewIndexOnCitextDatatypeIssue( | ||
objType, | ||
objName, | ||
"", | ||
) | ||
} else { | ||
queryIssue = NewPrimaryOrUniqueConstraintOnCitextDatatypeIssue( | ||
objType, | ||
objName, | ||
"", | ||
typeName, | ||
constraintName, | ||
) |
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.
you can use lo.Ternary for this.
queryissue = lo.Ternary(isPkorUk, NewPrimaryOrUniqueConstraintOnCitextDatatypeIssue(objType,objName,"",typeName,constraintName),NewIndexOnCitextDatatypeIssue(objType,objName,""))
Describe the changes in this pull request
Describe if there are any user-facing changes
Unsupported datatypes analyze schema report:

Callhome data for unsupported datatypes:

Schema Report for UK on complex datatypes:

Schema Report for PK on complex datatypes:

Callhome data for PK on complex datatypes:

How was this pull request tested?
Manually
Does your PR have changes that can cause upgrade issues?