Skip to content
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

fix view bug #100

Merged
merged 8 commits into from
Jun 3, 2024
Merged

fix view bug #100

merged 8 commits into from
Jun 3, 2024

Conversation

lsy3993
Copy link
Collaborator

@lsy3993 lsy3993 commented Jun 2, 2024

No description provided.

pkg/ccr/job.go Outdated
Comment on lines 949 to 966
/*
Creating table will only occur when sync db.
When create view, the db name of sql is source db name, we should use dest db name to create view
*/
createSql := createTable.Sql
viewRegex := regexp.MustCompile("(?i)^CREATE(\\s+)VIEW")
isCreateView := viewRegex.MatchString(createSql)
if isCreateView {
log.Debugf("create view, use dest db name to replace source db name")

// replace `internal`.`source_db_name`. to `internal`.`dest_db_name`.
originalName := "`internal`.`" + strings.TrimSpace(j.Src.Database) + "`."
dbNameRegex := regexp.MustCompile(originalName)
replaceName := "`internal`.`" + strings.TrimSpace(j.Dest.Database) + "`."
createTable.Sql = dbNameRegex.ReplaceAllString(createSql, replaceName)
log.Debugf("original create view sql is %s, after repalce, now sql is %s", createSql, createTable.Sql)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move SQL related operations into Spec.go:CreateTable, and add a parameter 'srcDatabase'.

createTable.Sql = dbNameRegex.ReplaceAllString(createSql, replaceName)

strings.ReplaceAll is enough here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


var results []string
// first, query information_schema.tables with table_schema and table_type, get all views' name
querySql := fmt.Sprintf("SELECT table_name FROM information_schema.tables WHERE table_schema = '%s' AND table_type = 'VIEW'", s.Database)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

desc ${tableName} all might be easier and more efficient than this.

See https://doris.apache.org/docs/1.2/query-acceleration/materialized-view/#query-materialized-views.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'desc ${tableName} all' don't support common view, materialized view is supported

pkg/ccr/job.go Outdated
if !allViewDeleted {
views, err := j.IDest.GetAllViewsFromTable(destTableName)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging errors and breaking here instead of returning, if you aim to loop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

pkg/ccr/job.go Outdated Show resolved Hide resolved
pkg/ccr/job.go Outdated Show resolved Hide resolved
pkg/ccr/job.go Outdated Show resolved Hide resolved
@w41ter w41ter merged commit 020743d into selectdb:dev Jun 3, 2024
1 check passed
@lsy3993 lsy3993 deleted the lsy_fix_view_bug branch June 3, 2024 13:11
lsy3993 added a commit that referenced this pull request Jun 7, 2024
lsy3993 added a commit to lsy3993/ccr-syncer that referenced this pull request Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants