-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[DO NOT MERGE] Testing #1500 #1518
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
Changes from all commits
e78c3a7
76a29d7
68d66eb
e94a555
39300cf
e018fbc
69d8158
7681d1c
b2aeab3
3033eb4
cc161a3
0522c75
71d3285
2ffe5e1
19208d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
name: gh-ost | ||
|
||
env: | ||
TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE: /var/run/docker.sock | ||
TESTCONTAINERS_RYUK_DISABLED: "true" | ||
|
||
up: | ||
- go: | ||
version: "1.22.12" | ||
- podman | ||
- custom: | ||
name: Go Dependencies | ||
met?: go mod download | ||
meet: echo 'go mod failed to download dependencies'; false | ||
|
||
commands: | ||
test: | ||
desc: Run all the tests. | ||
run: | | ||
export DOCKER_HOST=unix://$(podman machine inspect --format '{{.ConnectionInfo.PodmanSocket.Path}}') | ||
script/test |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1238,8 +1238,9 @@ func (this *Migrator) iterateChunks() error { | |||||
// When hasFurtherRange is false, original table might be write locked and CalculateNextIterationRangeEndValues would hangs forever | ||||||
|
||||||
hasFurtherRange := false | ||||||
expectedRangeSize := int64(0) | ||||||
if err := this.retryOperation(func() (e error) { | ||||||
hasFurtherRange, e = this.applier.CalculateNextIterationRangeEndValues() | ||||||
hasFurtherRange, expectedRangeSize, e = this.applier.CalculateNextIterationRangeEndValues() | ||||||
return e | ||||||
}); err != nil { | ||||||
return terminateRowIteration(err) | ||||||
|
@@ -1265,6 +1266,19 @@ func (this *Migrator) iterateChunks() error { | |||||
if err != nil { | ||||||
return err // wrapping call will retry | ||||||
} | ||||||
|
||||||
if this.migrationContext.PanicOnWarnings { | ||||||
if len(this.migrationContext.MigrationLastInsertSQLWarnings) > 0 { | ||||||
for _, warning := range this.migrationContext.MigrationLastInsertSQLWarnings { | ||||||
this.migrationContext.Log.Infof("ApplyIterationInsertQuery has SQL warnings! %s", warning) | ||||||
} | ||||||
if expectedRangeSize != rowsAffected { | ||||||
joinedWarnings := strings.Join(this.migrationContext.MigrationLastInsertSQLWarnings, "; ") | ||||||
terminateRowIteration(fmt.Errorf("ApplyIterationInsertQuery failed because of SQL warnings: [%s]", joinedWarnings)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The call to 'terminateRowIteration' is not returned, so its effect on the control flow is lost. Prepend 'return' to properly terminate the iteration when SQL warnings occur.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
atomic.AddInt64(&this.migrationContext.TotalRowsCopied, rowsAffected) | ||||||
atomic.AddInt64(&this.migrationContext.Iteration, 1) | ||||||
return nil | ||||||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -275,7 +275,7 @@ func BuildUniqueKeyRangeEndPreparedQueryViaOffset(databaseName, tableName string | |||
|
||||
uniqueKeyColumnNames := duplicateNames(uniqueKeyColumns.Names()) | ||||
uniqueKeyColumnAscending := make([]string, len(uniqueKeyColumnNames)) | ||||
uniqueKeyColumnDescending := make([]string, len(uniqueKeyColumnNames)) | ||||
uniqueKeyColumnDescending := make([]string, len(uniqueKeyColumnNames)) // TODO unused variable | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable 'uniqueKeyColumnDescending' is declared but never used. Remove it if it is not needed to avoid confusion.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||
for i, column := range uniqueKeyColumns.Columns() { | ||||
uniqueKeyColumnNames[i] = EscapeName(uniqueKeyColumnNames[i]) | ||||
if column.Type == EnumColumnType { | ||||
|
@@ -286,25 +286,46 @@ func BuildUniqueKeyRangeEndPreparedQueryViaOffset(databaseName, tableName string | |||
uniqueKeyColumnDescending[i] = fmt.Sprintf("%s desc", uniqueKeyColumnNames[i]) | ||||
} | ||||
} | ||||
joinedColumnNames := strings.Join(uniqueKeyColumnNames, ", ") | ||||
result = fmt.Sprintf(` | ||||
select /* gh-ost %s.%s %s */ | ||||
%s | ||||
from | ||||
%s.%s | ||||
where | ||||
%s and %s | ||||
%s, | ||||
(select count(*) from ( | ||||
select | ||||
%s | ||||
from | ||||
%s.%s | ||||
where | ||||
%s and %s | ||||
limit | ||||
%d | ||||
) select_osc_chunk) | ||||
from ( | ||||
select | ||||
%s | ||||
from | ||||
%s.%s | ||||
where | ||||
%s and %s | ||||
limit | ||||
%d | ||||
) select_osc_chunk | ||||
order by | ||||
%s | ||||
limit 1 | ||||
offset %d`, | ||||
databaseName, tableName, hint, | ||||
strings.Join(uniqueKeyColumnNames, ", "), | ||||
joinedColumnNames, joinedColumnNames, | ||||
databaseName, tableName, | ||||
rangeStartComparison, rangeEndComparison, | ||||
rangeStartComparison, rangeEndComparison, chunkSize, | ||||
joinedColumnNames, | ||||
databaseName, tableName, | ||||
rangeStartComparison, rangeEndComparison, chunkSize, | ||||
strings.Join(uniqueKeyColumnAscending, ", "), | ||||
(chunkSize - 1), | ||||
) | ||||
return result, explodedArgs, nil | ||||
// 2x the explodedArgs for the subquery (CTE would be possible but not supported by MySQL 5) | ||||
return result, append(explodedArgs, explodedArgs...), nil | ||||
} | ||||
|
||||
func BuildUniqueKeyRangeEndPreparedQueryViaTemptable(databaseName, tableName string, uniqueKeyColumns *ColumnList, rangeStartArgs, rangeEndArgs []interface{}, chunkSize int64, includeRangeStartValues bool, hint string) (result string, explodedArgs []interface{}, err error) { | ||||
|
@@ -342,8 +363,22 @@ func BuildUniqueKeyRangeEndPreparedQueryViaTemptable(databaseName, tableName str | |||
uniqueKeyColumnDescending[i] = fmt.Sprintf("%s desc", uniqueKeyColumnNames[i]) | ||||
} | ||||
} | ||||
|
||||
joinedColumnNames := strings.Join(uniqueKeyColumnNames, ", ") | ||||
result = fmt.Sprintf(` | ||||
select /* gh-ost %s.%s %s */ %s | ||||
select /* gh-ost %s.%s %s */ | ||||
%s, | ||||
(select count(*) from ( | ||||
select | ||||
%s | ||||
from | ||||
%s.%s | ||||
where | ||||
%s and %s | ||||
order by | ||||
%s | ||||
limit %d | ||||
) select_osc_chunk) | ||||
from ( | ||||
select | ||||
%s | ||||
|
@@ -353,17 +388,22 @@ func BuildUniqueKeyRangeEndPreparedQueryViaTemptable(databaseName, tableName str | |||
%s and %s | ||||
order by | ||||
%s | ||||
limit %d) select_osc_chunk | ||||
limit %d | ||||
) select_osc_chunk | ||||
order by | ||||
%s | ||||
limit 1`, | ||||
databaseName, tableName, hint, strings.Join(uniqueKeyColumnNames, ", "), | ||||
strings.Join(uniqueKeyColumnNames, ", "), databaseName, tableName, | ||||
databaseName, tableName, hint, joinedColumnNames, | ||||
joinedColumnNames, databaseName, tableName, | ||||
rangeStartComparison, rangeEndComparison, | ||||
strings.Join(uniqueKeyColumnAscending, ", "), chunkSize, | ||||
joinedColumnNames, databaseName, tableName, | ||||
rangeStartComparison, rangeEndComparison, | ||||
strings.Join(uniqueKeyColumnAscending, ", "), chunkSize, | ||||
strings.Join(uniqueKeyColumnDescending, ", "), | ||||
) | ||||
return result, explodedArgs, nil | ||||
// 2x the explodedArgs for the subquery (CTE would be possible but not supported by MySQL 5) | ||||
return result, append(explodedArgs, explodedArgs...), nil | ||||
} | ||||
|
||||
func BuildUniqueKeyMinValuesPreparedQuery(databaseName, tableName string, uniqueKey *UniqueKey) (string, error) { | ||||
|
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.
The key 'met?' is inconsistent with the key 'meet' on the following line, which may lead to configuration errors. Both keys should be identical if they are intended to represent the same check.
Copilot uses AI. Check for mistakes.