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

cli: debug.zip has extra unnecessary hex_ columns #112032

Closed
cucaroach opened this issue Oct 9, 2023 · 0 comments · Fixed by #112033
Closed

cli: debug.zip has extra unnecessary hex_ columns #112032

cucaroach opened this issue Oct 9, 2023 · 0 comments · Fixed by #112033
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@cucaroach
Copy link
Contributor

cucaroach commented Oct 9, 2023 •
edited by cockroach-jira-scripts

Ever since #80398 was fixed we haven't needed the extra hex_ columns in various system table output in the debug.zip. We should remove them.

Jira issue: CRDB-32204
@cucaroach cucaroach added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Oct 9, 2023
@cucaroach cucaroach mentioned this issue Oct 9, 2023
Merged
craig bot pushed a commit that referenced this issue Oct 10, 2023
@cucaroach @Xiang-Gu
@mgartner @herkolategan
Merge #112033 #112040 #112067 #112074 #112086 #112096 #112097
d1981cd 
112033: cli: remove redundant hex_ columns in debug.zip output r=rickystewart a=cucaroach

In #80398 we fixed the CLI to respect bytea_output
session variable and properly output BYTES columns in \xDEADBEEF format.
Now we don't need the extra hex_ columns. This makes debug.zips smaller
and quicker to create (jobs payloads can get big and so can descriptors
for large tables).

In order not to break cockroach debug decode-proto extend it to look
for and remove the \x prefix when attempting to decode hex.

Fixes: #112032
Epic: none
Release note (cli change): debug zip's no longer include redundant hex_
columns for system table BYTES columns.


112040: sctest: Skip "crdb_internal" entry in comparator testing from logictest corpus r=Xiang-Gu a=Xiang-Gu

Statements collected from `crdb_internal` is of little interest to us
for the purpose of schema changer comparator testings, as it contains
only `crdb_internal` functions. Furthermore, there are statements that
will crash our framework, such as `SELECT crdb_internal.force_panic('foo')`.

This commit therefore adds a check to skip it. This check will also home
any future skipped entries for whatever reason.

Epic: None
Release note: None

112067: sql: fix minor bugs and add tests for procedures r=mgartner a=mgartner

#### sql: add tests for procedures with anonymous arguments

Release note: None

Epic: CRDB-25388

#### sql: fix `CREATE OR REPLACE PROCEDURE` and add tests

This commit fixes a minor bug with `CREATE OR REPLACE PROCEDURE` that
allowed it to replace functions. The bug also allowed
`CREATE OR REPLACE FUNCTION` to replace procedures. Tests for
`CREATE OR REPLACE PROCEDURE` have also been added.

Release note: None

#### sql: fix routine-related error messages

This commit fixes some minor bugs with the error message of
routine-related statements.

Release note: None


112074: optbuilder: move insert fast path test to optbuilder r=msirek a=msirek

This commit moves an insert fast path uniqueness check test case involving a volatile expression to a more applicable BuilderTest.

Epic: CRDB-26290
Informs: #58047

Release note: None

112086: serverutils: use options for SQL connections r=knz a=herkolategan

Previously `ApplicationLayerInterface` contained multiple `SQLConn` method variants for passing connection parameters. Various tests throughout the code use `SQLConn` or `PGUrl` to connect, but the latter does not take into account virtual clusters unless informed.

This PR is meant to enable `SQLConn` to become a standard replacement for `PGUrl` in order to leverage the probabilistic nature of the tests which select between running a second virtual cluster or not.

Numerous tests still use `PGUrl` and pass user, password, or cert details. These tests will soon be updated to start using `SQLConn` and hence should have the same functionality available. This enables the test to not have to deal with querying its probabilistic mode to determine if it should pass a virtual cluster name to `PGUrl`. The `ApplicationLayerInterface` already handles this logic and tests should leverage this.

See also: #111134
Epic: CRDB-31933

Release note: None

112096: optbuilder: move finishBuildLastStmt to routine.go r=mgartner a=mgartner

When routine-related optbuilder logic was moved to a new file,
`routine.go`, the `finishBuildLastStmt` function was missed. This commit
corrects that mistake.

Epic: CRDB-25388

Release note: None


112097: sql/logictest: add CALL statements to drop_procedure r=mgartner a=mgartner

This commit improves the coverage of tests in `drop_procedure` by adding
`CALL` statements to ensure that dropped procedures cannot be executed.

Epic: CRDB-25388

Release note: None


Co-authored-by: Tommy Reilly <treilly@cockroachlabs.com>
Co-authored-by: Xiang Gu <xiang@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Mark Sirek <sirek@cockroachlabs.com>
Co-authored-by: Herko Lategan <herko@cockroachlabs.com>
@craig craig bot closed this as completed in 31bb6c3 Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cli: remove redundant hex_ columns in debug.zip output cucaroach/cockroach
1 participant
@cucaroach