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

sql: implement create index inside the new schema changer #67264

Closed
fqazi opened this issue Jul 6, 2021 · 0 comments · Fixed by #67266 or #72610
Closed

sql: implement create index inside the new schema changer #67264

fqazi opened this issue Jul 6, 2021 · 0 comments · Fixed by #67266 or #72610
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@fqazi
Copy link
Collaborator

fqazi commented Jul 6, 2021

Add support for CREATE INDEX inside the schema changer, as a first step to adding other CREATE operations
into the new schema changer.

@fqazi fqazi added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 6, 2021
@fqazi fqazi self-assigned this Jul 6, 2021
fqazi added a commit to fqazi/cockroach that referenced this issue Jul 26, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Jul 27, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Jul 30, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Aug 3, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Aug 4, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Aug 9, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Aug 25, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 3, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 4, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 8, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 8, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 8, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
craig bot pushed a commit that referenced this issue Nov 9, 2021
67266: sql: add support for create index inside new schema changer r=fqazi a=fqazi

Fixes: #67264 

Previously, create index was not supported inside the
new schema changer. This was inadequate because to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
@craig craig bot closed this as completed in 0bdf2a7 Nov 9, 2021
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 10, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 10, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 11, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 11, 2021
Fixes: cockroachdb#67264

Previously, create index was not supported inside the
new shcema changer. This was inadequate because, to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None
craig bot pushed a commit that referenced this issue Nov 12, 2021
72590: ci: add bazel-based url check job for ci r=rail a=rickystewart

Closes #67336.

Release note: None

72610: sql: add support for create index inside new schema r=fqazi a=fqazi

Fixes: #67264

Previously, create index was not supported inside the
new schema changer. This was inadequate because to
help support transaction schema changes in the future
we need all schema changes inside the new schema changer.
To address this, this patch adds CREATE INDEX inside
the new schema changer.

Release note: None

72624: backupccl: fix span-after-finish and some other races in processors r=andreimatei a=andreimatei

See individual commits.
This is all happening because I want to make using Spans after they've been FInish()ed less tolerated than it has historically been.

72629: tracing: improve the ContextWithRecordingSpan interface r=andreimatei a=andreimatei

Before this patch, it was technically illegal to call the callback
returned by ContextWithRecordingSpan multiple times, because it would
Finish() the respective span multiple times. This patch makes it legal.

Calling the callback multiple times is nice because it allows patterns
like:

  ..., cancel := ContextWithRecordingSpan(...)
  defer cance()
  ...
  cancel()
  ...

This allows the respective span to be closed as early as possible. At
least one test uses this pattern, and is currently getting away with it
because double finish is tolerated. But double finish won't be
tolerated for long, and so this patch adds extra precautions in order to
keep the pattern working.

This patch also changes the ContextWithRecordingSpan() interface. The
function used to return two callbacks - one for finishing the span, one
for collecting the recording. This was overkill, and pretty confusing.
The finish callback would also cancel the context, although nobody seems
to need that. There needed to be an order for calling the two callbacks,
which seemed complicated. Also, the allowed order (getRecording() first,
finish() second) is not great because the recording you get shows an
unfinished root span. This patch unifies the callbacks into a single
one.

Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
1 participant