distsqlrun: explain(distsql) queries w/ fragment #27858
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jordanlewis
requested review from
solongordon,
asubiotto,
RaduBerinde and
a team
|
This change is |
Previously, explain(distsql) emitted a URL that used the query string to send the compressed plan data to cockroachdb.github.io. This was unideal, as query strings are transmitted to the server, but these query strings might contain compressed, sensitive data. Now, the query string is sent in the URL fragment, which is kept on the client side by the browser. Release note: None
|
I (accidentally) pushed a change directly to The only change I manually made in this PR was to change the output URL to use Fragment instead of RawQuery. The rest were done with a sed that replaced |
|
This is very cool! Great idea! LGTM I just looked at your commit in that other repo, that also LGTM. |
|
Thanks for the quick review! |
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Jul 23, 2018
27809: importccl: pre-read schemas in mysqldump import r=dt a=dt This switches mysqldump import to use read schemas from during setup on the gateway, rather than during sampling, similar to how pgdump operates (i.e. in three passes over the input rather than two). This simplifies handling foreign keys, which can sometimes appear in a table definition before the table they reference — making them hard to correctly resolve immediately. Reading though the whole file to capture all the schemas before evaluating them should make that a bit easier. In the future, a return to 2-pass could be possible either if it turns out that KVs can be produced correctly even if the schema is later changed by a foreign key, or by oversampling raw rows of the input *without* converting during the read extracting the schemas, then using those schemas to convert the sampled rows to KVs from which the splits can be sampled. Release note: none. 27858: distsqlrun: explain(distsql) queries w/ fragment r=jordanlewis a=jordanlewis Previously, explain(distsql) emitted a URL that used the query string to send the compressed plan data to cockroachdb.github.io. This was unideal, as query strings are transmitted to the server, but these query strings might contain compressed, sensitive data. Now, the query string is sent in the URL fragment, which is kept on the client side by the browser. Release note: None Co-authored-by: David Taylor <[email protected]> Co-authored-by: Jordan Lewis <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, explain(distsql) emitted a URL that used the query string to
send the compressed plan data to cockroachdb.github.io. This was
unideal, as query strings are transmitted to the server, but these query
strings might contain compressed, sensitive data.
Now, the query string is sent in the URL fragment, which is kept on the
client side by the browser.
Release note: None