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.

Sign up for GitHub

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

Jump to bottom

sql/schemachanger: use declarative schemachange for add column sequence expr #81781

Closed
fqazi opened this issue May 24, 2022 · 0 comments · Fixed by #81782
Closed

sql/schemachanger: use declarative schemachange for add column sequence expr #81781

fqazi opened this issue May 24, 2022 · 0 comments · Fixed by #81782
Assignees
@fqazi
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@fqazi
Copy link
Collaborator

fqazi commented May 24, 2022 •
edited by cockroach-jira-scripts
Loading

Currently when adding columns with the declarative schema changer, we fallback to the legacy schema changer when sequence expressions are observed inside newly added columns this was done because:

  1. We didn't support properly surfacing errors from the backfiller (this was addressed in the original PR)
  2. We lacked support for updating error related telemetry during executing

Once these two points are addressed we can enable support for the declarative schema changer for all expressions.

Jira issue: CRDB-16067
@fqazi fqazi added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-schema-deprecated Use T-sql-foundations instead labels May 24, 2022
@fqazi fqazi self-assigned this May 24, 2022
@fqazi fqazi mentioned this issue May 24, 2022
Merged
fqazi added a commit to fqazi/cockroach that referenced this issue May 25, 2022
@fqazi
sql: use declarative schemachange for add column sequence exprs
a0836b4 
Fixes: cockroachdb#81781

Previously, the declarative schema changer was disabled for
add column expressions with sequence references (i.e. default,
on update, computed) because we were missing telemetry
from the legacy schema changer for during backfill related failures.
This was inadequate because the lack of telemetry from these
areas could be seen as a usability issue. To address this,
this patch adds in support for missing telemetry and enables
supports for add column operations with sequence operations.

Release note: None

Release note (<category, see below>): <what> <show> <why>
fqazi added a commit to fqazi/cockroach that referenced this issue May 26, 2022
@fqazi
sql: use declarative schemachange for add column sequence exprs
01c2118 
Fixes: cockroachdb#81781

Previously, the declarative schema changer was disabled for
add column expressions with sequence references (i.e. default,
on update, computed) because we were missing telemetry
from the legacy schema changer for during backfill related failures.
This was inadequate because the lack of telemetry from these
areas could be seen as a usability issue. To address this,
this patch adds in support for missing telemetry and enables
supports for add column operations with sequence operations.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Jun 2, 2022
@fqazi
sql: use declarative schemachange for add column sequence exprs
f9bd08e 
Fixes: cockroachdb#81781

Previously, the declarative schema changer was disabled for
add column expressions with sequence references (i.e. default,
on update, computed) because we were missing telemetry
from the legacy schema changer for during backfill related failures.
This was inadequate because the lack of telemetry from these
areas could be seen as a usability issue. To address this,
this patch adds in support for missing telemetry and enables
supports for add column operations with sequence operations.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Jun 6, 2022
@fqazi
sql: use declarative schemachange for add column sequence exprs
2268b1e 
Fixes: cockroachdb#81781

Previously, the declarative schema changer was disabled for
add column expressions with sequence references (i.e. default,
on update, computed) because we were missing telemetry
from the legacy schema changer for during backfill related failures.
This was inadequate because the lack of telemetry from these
areas could be seen as a usability issue. To address this,
this patch adds in support for missing telemetry and enables
supports for add column operations with sequence operations.

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Jun 7, 2022
@fqazi
sql: use declarative schemachange for add column sequence exprs
760f93a 
Fixes: cockroachdb#81781

Previously, the declarative schema changer was disabled for
add column expressions with sequence references (i.e. default,
on update, computed) because we were missing telemetry
from the legacy schema changer for during backfill related failures.
This was inadequate because the lack of telemetry from these
areas could be seen as a usability issue. To address this,
this patch adds in support for missing telemetry and enables
supports for add column operations with sequence operations.

Release note: None
craig bot pushed a commit that referenced this issue Jun 7, 2022
@fqazi @rafiss
@erikgrinaker @rickystewart @maryliag @rytaft
Merge #81782 #82388 #82451 #82466 #82476 #82519
02cc393 
81782: sql: use declarative schemachange for add column sequence exprs r=fqazi a=fqazi

Fixes: #81781

Previously, the declarative schema changer was disabled for
add column expressions with sequence references (i.e. default,
on update, computed) because we were missing telemetry
from the legacy schema changer for during backfill related failures.
This was inadequate because the lack of telemetry from these
areas could be seen as a usability issue. To address this,
this patch adds in support for missing telemetry and enables
supports for add column operations with sequence operations.

Release note: None

82388: sql: remove date/intervalstyle_enabled from code r=otan,mgartner a=rafiss

fixes #81529
 
No release note is needed, since v22.1 already included a release note
about how these are both hardcoded to true and cannot be changed.

The session variables and cluster settings remain in the code, but are
marked as retired.

Release note: None

82451: kvserver/rangefeed: fix off-by-one in `NewCatchUpIterator()` r=miretskiy a=erikgrinaker

**kvserver/rangefeed: fix off-by-one in NewCatchUpIterator()**

`NewCatchUpIterator` created the `MVCCIncrementalIterator` with
`RangeFeedRequest.Header.Timestamp.Prev()`, because it assumed the given
timestamp was inclusive. However, the rest of the rangefeed code treats
this timestamp as exclusive.

This patch uses the correct, exclusive timestamp when creating the
`MVCCIncrementalIterator`. This change has no externally visible effect:
even though the previous code would cause `MVCCIncrementalIterator` to emit
keys at `Timestamp`, the `CatchUpScan()` method would discard those
events if they were at or below `Timestamp`.

An integration test is also added to verify that the start timestamp of
a rangefeed catchup scan is exclusive. The test passes both with the new
and the old code, as expected.

Release note: None

**rangefeed: emphasize that start time is exclusive**

This patch adds comments for rangefeed-related APIs and components
emphasizing that the start timestamp of rangefeeds is exclusive. In
other words, the first possible emitted event (including catchup scans)
will be at `RangeFeedRequest.Header.Timestamp.Next()`. Several
parameters are also renamed to emphasize this.

There are no functional or semantic changes.

Touches #82488.

Release note: None

82466: dev: have `doctor` advise to set a particular `tmpdir` r=rail a=rickystewart

Bazel's default behavior of rooting the `tmpdir` to an "in-sandbox"
directory has been a point of confusion for CRL developers. The sandbox
directory does not exist after the test is run (unless `--sandbox_debug`
is provided), which is sometimes confusing for folks who expect their
test's temp files to be present where the logs suggest they should be
(see #82413). Furthermore, the long `tmpdir` used in these cases breaks
tests that create Unix sockets on OS's where Unix sockets have a maximum
path length.

Avoid these problems by having `doctor` just tell you to manually set a
`test_tmpdir`. We add `/tmp` to `gitignore` in case people want to root
it at the `tmp` directory in their checkout.

Closes #72918.
Closes #82413.

Release note: None

82476: update cluster-ui to v22.2.0-prerelease-1 r=maryliag a=maryliag

Update cluster-ui to latest published version

Release note: None

82519: stats: fix flaky test TestDefaultColumns r=rytaft a=rytaft

TestDefaultColumns creates statistics on a table with 110 columns
using the command `CREATE STATISTICS s FROM t.a`. It then checks
that there are exactly 101 column statistics on table t.a with
statistics_name = 's' (one stat for the primary index, plus 100
other column stats). However, this test may be flaky if automatic
statistics are running, since each new automatic stat will cause
other stats to be deleted.

Although the test disables automatic stats at the beginning, it seems
that some sort of race condition may cause it to be reenabled. This
commit fixes the problem by disabling automatic stats using the new table
level settings, ensuring that the 101 column stats are not deleted after
they have been created.

Fixes #81513

Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Rebecca Taft <[email protected]>
@craig craig bot closed this as completed in 9a9fe00 Jun 7, 2022
@healthy-pod healthy-pod added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fqazi fqazi

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: use declarative schemachange for add column sequence exprs fqazi/cockroach
2 participants
@fqazi @healthy-pod