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/row: TestJobBackedSeqChunkProvider incorrectly uses keys.TODOSQLCodec #82886

Closed
knz opened this issue Jun 14, 2022 · 1 comment · Fixed by #82890
Closed

sql/row: TestJobBackedSeqChunkProvider incorrectly uses keys.TODOSQLCodec #82886

knz opened this issue Jun 14, 2022 · 1 comment · Fixed by #82890
Assignees
Labels
A-disaster-recovery A-multitenancy Related to multi-tenancy A-testing Testing tools and infrastructure C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery

Comments

@knz
Copy link
Contributor

knz commented Jun 14, 2022

Forked from #48123.

Describe the problem

In TestJobBackedSeqChunkProvider, (sql/row/expr_walker_test.go), an eval.Context is instantiated near the top with keys.TODOSQLCodec. This is incorrect.

Expected behavior

Instead, the test should bind the eval.Context to the SQLCodec coming from the TestServer.

(After #76582, this will be automatically and probabilistically assinged to either a system tenant codec or a secondary tenant codec.)

cc @adityamaru for triage

Jira issue: CRDB-16712

@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-disaster-recovery A-testing Testing tools and infrastructure A-multitenancy Related to multi-tenancy T-disaster-recovery labels Jun 14, 2022
@blathers-crl
Copy link

blathers-crl bot commented Jun 14, 2022

cc @cockroachdb/bulk-io

@adityamaru adityamaru self-assigned this Jun 14, 2022
craig bot pushed a commit that referenced this issue Jun 29, 2022
82890: row: remove TODOSQLCodec in TestJobBackedSeqChunkProvider r=knz a=adityamaru

Fixes: #82886

Release note: None

83252: build: add configurations for Antithesis build instrumentation r=AlexTalks a=AlexTalks

This adds the `instrument` and `instrumentshort` targets to our
Makefile-based builds to produce a CockroachDB binary instrumented with
the Antithesis Go Instrumentation Package. The configurations, which
require pre-installation of this instrumentation package, are used in
the process of testing using the Antithesis autonomous testing system,
with the instrumented code reporting coverage information to the
Antithesis fuzzer.

As the Antithesis instrumentor requires outputting the generated code to
a location outside the repository, a temporary location must be supplied
via the `INSTRUMENTATION_TMP` parameter. This temporary location will also
contain the generated symbol files.

To run:
```
make instrumentshort INSTRUMENTATION_TMP=$HOME/tmp/antithesis
./cockroach-instrumented --help
```

Release note: None

83386: batcheval: handle MVCC range tombstones in `RefreshRange` r=aliher1911 a=erikgrinaker

This patch add handling of MVCC range tombstones in `RefreshRange`, by
erroring on them as a committed value.

Resolves #83383.

Release note: None

Co-authored-by: Aditya Maru <adityamaru@gmail.com>
Co-authored-by: Alex Sarkesian <asarkesian@gmail.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
@craig craig bot closed this as completed in aa926db Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery A-multitenancy Related to multi-tenancy A-testing Testing tools and infrastructure C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants