sql: plumb an error in the distsql planner #24688
Conversation
We got a panic inside `getTypesForPlanResult` via Sentry; unfortunately I cannot reproduce the panic (I believe what triggers it is the type of a placeholder passed during prepare, but we don't have that information). This change addresses a TODO to make this condition result in an error instead of a panic. Updates cockroachdb#24680. Release note (bug fix): Converted a panic related to an unsupported type to an error.
|
This change is |
|
Code Reviewed 2 of 2 files at r1. Comments from Reviewable |
|
I don't know how to cause an error in this path.. If I knew, I would fix that :) Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed. Comments from Reviewable |
|
Ah I didn't realize you mentioned that in the PR description. cc @justinj, didn't you add some testing around this recently? If this method isn't tested for completeness over all types, it should be. Are you aware of any times that don't have an output for this method? |
|
Hm, yeah, I did add a test for |
|
well, shrug. Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed. Comments from Reviewable |
|
bors I mean, bors r+ Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed. Comments from Reviewable |
Build failed (retrying...) |
24683: stats: consolidate the stat and histo caches r=RaduBerinde a=RaduBerinde The stats cache was implemented as a pair of caches: one where each entry is a table, holding all the stats but without histograms; and another which holds individual histograms. The idea here was that histograms are bigger so we would be able to store more table stats if we limited the number of histograms in the cache. Unfortunately, this interface is problematic to use when we will add code to automatically remove old stats: in-between getting the stats and getting a corresponding histogram, the stat can disappear leading to an error. Another problem is that with a cold cache, we do a separate query for each histogram. Simplifying to a single cache that stores all the stats, including histograms. We can revisit this later if we find that we need a large cache size and memory usage becomes a problem. Plus a minor improvement: we now return the TableStatistics in new-to-old order. Release note: None 24688: sql: plumb an error in the distsql planner r=RaduBerinde a=RaduBerinde We got a panic inside `getTypesForPlanResult` via Sentry; unfortunately I cannot reproduce the panic (I believe what triggers it is the type of a placeholder passed during prepare, but we don't have that information). This change addresses a TODO to make this condition result in an error instead of a panic. Updates #24680. Release note (bug fix): Converted a panic related to an unsupported type to an error. 24691: workloadccl/allccl: skip tpcc validation test under testshort and testrace r=danhhz a=danhhz It loads a lot of data and so it takes too long to run under short or race tests. Cutting the data down would make the test uninteresting since the whole point of the test is to check that the validation function agrees with the initial data generation, so just skip it in these cases. Closes #24660 #24659 Release note: None Co-authored-by: Radu Berinde <radu@cockroachlabs.com> Co-authored-by: Daniel Harrison <daniel.harrison@gmail.com>
Build succeeded |
We got a panic inside
getTypesForPlanResultvia Sentry;unfortunately I cannot reproduce the panic (I believe what triggers
it is the type of a placeholder passed during prepare, but we don't
have that information).
This change addresses a TODO to make this condition result in an error
instead of a panic.
Updates #24680.
Release note (bug fix): Converted a panic related to an unsupported
type to an error.