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

testcluster: don't overwrite localities indiscriminately #40451

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

andreimatei
Copy link
Contributor

Before this patch, a TestCluster would set localities for all nodes to a
static values. This was overwriting any values set throught the
TestClusterArgs. This patch makes it so that, if any localities are set
in the args, we don't overwrite them.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@rohany rohany left a comment

Choose a reason for hiding this comment

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

Ah nice! You saw my annoyed comment in that test. This looks good to me, but wait for someone else who knows the code better.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball, @darinpp, and @rohany)

Copy link
Contributor

@darinpp darinpp left a comment

Choose a reason for hiding this comment

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

nice

@andreimatei
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 4, 2019

Build failed

Copy link
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

flake #40362

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball)

craig bot pushed a commit that referenced this pull request Sep 5, 2019
40436: sql: more test fixes for opt-driven foreign keys r=RaduBerinde a=RaduBerinde

Fixing up some expected errors in tests and making sure we don't
buffer the mutation input if we fall back to the legacy path (the
bufferNode is unnecessary and interferes with an interleaved delete
fast path).

Release note: None

40451: testcluster: don't overwrite localities indiscriminately r=andreimatei a=andreimatei

Before this patch, a TestCluster would set localities for all nodes to a
static values. This was overwriting any values set throught the
TestClusterArgs. This patch makes it so that, if any localities are set
in the args, we don't overwrite them.

Release note: None

40485: sql: Fix a bug with ordinal_position in information_schema.columns r=arulajmani a=arulajmani

When a column other than the last is dropped, ordinal_position in
information_schema.columns virtual table no longer matches attnum from
the pg_attribute table. This PR fixes this issue.

Fixes #39787

Release note (bug fix): ordinal_position in information_schema.columns
matches pg_attribute.attnum after a column is dropped.

40511: exec: fix explain(vec) for queries with subqueries r=jordanlewis a=jordanlewis

Also add logic tests that show the explain(vec) plans for all of the
tpch queries.

Closes #40484.

Release note: None

Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
Co-authored-by: Arul Ajmani <arula@cockroachlabs.com>
Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
@craig
Copy link
Contributor

craig bot commented Sep 5, 2019

Build failed (retrying...)

@craig
Copy link
Contributor

craig bot commented Sep 5, 2019

Merge conflict (retrying...)

@craig
Copy link
Contributor

craig bot commented Sep 5, 2019

Merge conflict

Before this patch, a TestCluster would set localities for all nodes to a
static values. This was overwriting any values set throught the
TestClusterArgs. This patch makes it so that, if any localities are set
in the args, we don't overwrite them.

Release note: None
Copy link
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball)

craig bot pushed a commit that referenced this pull request Sep 6, 2019
40451: testcluster: don't overwrite localities indiscriminately r=andreimatei a=andreimatei

Before this patch, a TestCluster would set localities for all nodes to a
static values. This was overwriting any values set throught the
TestClusterArgs. This patch makes it so that, if any localities are set
in the args, we don't overwrite them.

Release note: None

Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented Sep 6, 2019

Build succeeded

@craig craig bot merged commit 4577e13 into cockroachdb:master Sep 6, 2019
@bobvawter
Copy link
Member

bobvawter commented Mar 10, 2020

CLA assistant check
All committers have signed the CLA.

@andreimatei andreimatei deleted the testcluster.localities branch April 24, 2020 18:57
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.

5 participants