-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
sql: different schema update logic in tests and real sessions #17347
Comments
@knz I think this is happening because you're using the Executor to send statements. I don't exactly know why the Executor won't wait for schema changes when called internally at the moment - perhaps that's something we need to fix. Anyway, a workaround is to use the sqlDB return value of |
I changed my test code to use |
Note that the same test not using an index works fine, i.e. db and table descriptors are properly created/updated/published. The problem is specific to indexes. |
|
The distsql error happens in both cases, both the successful and erroneous cases. IT is expected, distsql doesn't run the CREATE statements. |
@vivekmenezes that always gets printed by any query plan that isn't distsql enabled, I think it's normal. Looks like this is a problem others have hit as well - check out what
|
I'll take a look at this issue later today
…On Tue, Aug 1, 2017 at 11:20 AM Jordan Lewis ***@***.***> wrote:
@vivekmenezes <https://github.com/vivekmenezes> that always gets printed
by any query plan that isn't distsql enabled, I think it's normal.
Looks like this is a problem others have hit as well - check out what
backup_test.go has to do:
sqlDB.Exec(`CREATE INDEX balance_idx ON data.bank (balance)`)
testutils.SucceedsSoon(t, func() error {
var unused string
var createTable string
sqlDB.QueryRow(`SHOW CREATE TABLE data.bank`).Scan(&unused, &createTable)
if !strings.Contains(createTable, "balance_idx") {
return errors.New("expected a balance_idx index")
}
return nil
})
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17347 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ALOpBEFMfVYHInC-CSp1pzvNwhNOijZfks5sT0IxgaJpZM4Op2RL>
.
|
(I solved my problem in #17310 by creating the index and the table together, i.e. |
I bet the statement is accidentally getting routed through the asynchronous path. |
works. Can you attach an example that doesn't |
I thought the example above was sufficient. I was observing this in the branch #17310 forked from, perhaps another commit on master fixed the problem already. I will try again after rebasing. |
Ok I found the error! My test was incorrectly skipping the migration responsible for creating the I am surprised that the lack of |
I am running the following statements:
When I run this in a regular session, the SELECT works fine, and I observe the following server log:
When run in a test however, (TestServer with manually created session) I observe the following trace instead, with an error:
It is as if the schema change is not fully executed. How do I ensure the schema change is complete (incl backfill) when running a test?
My test code looks like this:
Note that the same problem persists whether I run all the statements in a single call to ExecuteStatements, or using 1 statement per call.
What am I missing? Do I need to initialize my test differently?
cc @jordanlewis @vivekmenezes
(found via #17310)
The text was updated successfully, but these errors were encountered: