-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
exec: Fix test case TestTraceFieldDecomposition. #39027
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the expected and the actual output before this change (if you happen to have it somewhere)?
Reviewed 1 of 1 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @solongordon)
The test fails here because the automatic string conversion of the core union has a whitespace at the end of it, which is one thing that the test is checking for -- this makes sure that this isn't the case. |
bors r+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @rohany)
pkg/sql/distsqlrun/column_exec_setup.go, line 500 at r1 (raw file):
default: return nil, nil, memUsage, errors.Newf("unsupported processor core %s", strings.TrimSpace(core.String()))
Hm, why doesn't the equivalent line in processors.go
need a call to TrimSpace
?
cockroach/pkg/sql/distsqlrun/processors.go
Line 1199 in e2e2c67
return nil, errors.Errorf("unsupported processor core %s", core) |
Does the difference come from using
errors.Newf
in one place and errors.Errorf
in the other? Not really a big deal but we might as well make the code match.
bors r- |
Canceled |
Fix test case TestTraceFieldDecomposition, addressed in cockroachdb#38935 Release note: None
bors r+ |
38971: exec: Add more tracing output to cfetcher. r=rohany a=rohany The previous commit adding logging to the cfetcher did not add enough logging statements to match that of the rowfetcher. This PR adds these missing statements. When rebased on top of #38945, this fixes most of the failing tests, except for a few which we think might not be compatible with the vectorized engine. Release note: None 38973: opt: use URL for EXPLAIN (OPT,ENV) r=RaduBerinde a=RaduBerinde The output of `EXPLAIN (OPT,ENV)` is usually unwieldy and can be a pain to be passed around properly (reformatting/terminal issues). This change converts it to an encoded URL, similar to distributed plans. The URL is to a simple page that decodes the compressed data and displays it. Fixes #38942. Example URL: https://cockroachdb.github.io/text/decode.html#eJxUzs2OqjAcBfC1fYp_3AgJRSBaUFeINeGmooFec83NzU0LfjTDgKkwozsfgif0SSYks5nlSc755eyP-qbqag5Rnb_pWuSX1RKiiMGHO7M928GivF6E7TnuzCEOwcHUweepfyLEnx3zCS6Ubh5g3APyn0xwqar2js9Va4FsVdlAvxs7_tgNwHPm02Du-Raca9d2PZuYCEUpDTkFHi4ZBSHBQAMBccIDSH4zZqGB_JHW4SZmBxhetXoX-jEEQ1ggLdD1pypMZC4QChmn6TdYHE-iLZtC2tdWliq3Rc_9ohGHjIc8zngcZTD6-2-0QIj-2bEwTsDY7rgFNNmbkFHWdyWs0-2mv7dNVzSF5QHEAmGMMbrVukHw6rpX93x1T7jlogIh0VcAAAD__1ttYDI= Release note (sql change): EXPLAIN (OPT,ENV) now returns a URL with the data encoded in the fragment portion. Opening the URL shows a page with the decoded data. Note that the data is processed in the local browser session and is never sent out. 39027: exec: Fix test case TestTraceFieldDecomposition. r=rohany a=rohany Fix test case TestTraceFieldDecomposition, addressed in #38935 Release note: None Co-authored-by: Rohan Yadav <[email protected]> Co-authored-by: Radu Berinde <[email protected]>
Build succeeded |
Fix test case TestTraceFieldDecomposition, addressed in #38935
Release note: None