Skip to content

Commit

Permalink
fix(sqlsmith): skip division by zero error (risingwavelabs#6599)
Browse files Browse the repository at this point in the history
* update validation of errors

* test skipped query counts

* docs, naming

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
kwannoel and mergify[bot] authored Nov 26, 2022
1 parent 0128ff1 commit 4b55926
Showing 1 changed file with 63 additions and 6 deletions.
69 changes: 63 additions & 6 deletions src/tests/sqlsmith/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ use tokio_postgres::error::{DbError, Error as PgError, SqlState};

use crate::{create_table_statement_to_table, mview_sql_gen, parse_sql, sql_gen, Table};

/// e2e test runner for sqlsmith
pub async fn run(client: &tokio_postgres::Client, testdata: &str, count: usize) {
let mut rng = rand::rngs::SmallRng::from_entropy();
let (tables, mviews, setup_sql) = create_tables(&mut rng, testdata, client).await;

// Test sqlsmith first
test_sqlsmith(client, &mut rng, tables.clone(), &setup_sql).await;
tracing::info!("Passed sqlsmith tests");

// Test batch
// Queries we generate are complex, can cause overflow in
// local execution mode.
Expand All @@ -49,6 +54,47 @@ pub async fn run(client: &tokio_postgres::Client, testdata: &str, count: usize)
drop_tables(&mviews, testdata, client).await;
}

/// Sanity checks for sqlsmith
pub async fn test_sqlsmith<R: Rng>(
client: &tokio_postgres::Client,
rng: &mut R,
tables: Vec<Table>,
setup_sql: &str,
) {
// Test percentage of skipped queries <=5% of sample size.
let mut batch_skipped = 0;
let batch_sample_size = 100;
client
.query("SET query_mode TO distributed;", &[])
.await
.unwrap();
for _ in 0..batch_sample_size {
let sql = sql_gen(rng, tables.clone());
tracing::info!("Executing: {}", sql);
let response = client.query(sql.as_str(), &[]).await;
batch_skipped +=
validate_response_with_skip_count(setup_sql, &format!("{};", sql), response);
}
if batch_skipped > 5 {
panic!("skipped >5% batch queries");
}

let mut stream_skipped = 0;
let stream_sample_size = 100;
for _ in 0..stream_sample_size {
let (sql, table) = mview_sql_gen(rng, tables.clone(), "stream_query");
tracing::info!("Executing: {}", sql);
let response = client.execute(&sql, &[]).await;
stream_skipped +=
validate_response_with_skip_count(setup_sql, &format!("{};", sql), response);
drop_mview_table(&table, client).await;
}

if stream_skipped > 5 {
panic!("skipped >5% stream queries");
}
}

fn get_seed_table_sql(testdata: &str) -> String {
let seed_files = vec!["tpch.sql", "nexmark.sql"];
seed_files
Expand Down Expand Up @@ -136,24 +182,35 @@ fn is_broken_chan_err(db_error: &DbError) -> bool {
.contains("internal error: broken fifo_channel")
}

/// Certain errors are permitted to occur. This is because:
/// 1. It is more complex to generate queries without these errors.
/// 2. These errors seldom occur, skipping them won't affect overall effectiveness of sqlsmith.
fn is_permissible_error(db_error: &DbError) -> bool {
let is_internal_error = *db_error.code() == SqlState::INTERNAL_ERROR;
is_internal_error
&& (is_numeric_out_of_range_err(db_error)
|| is_broken_chan_err(db_error)
|| is_division_by_zero_err(db_error))
(is_internal_error && is_broken_chan_err(db_error))
|| is_numeric_out_of_range_err(db_error)
|| is_division_by_zero_err(db_error)
}

/// Validate client responses
fn validate_response<_Row>(setup_sql: &str, query: &str, response: Result<_Row, PgError>) {
validate_response_with_skip_count(setup_sql, query, response);
}

/// Validate client responses, returning a count of skipped queries.
fn validate_response_with_skip_count<_Row>(
setup_sql: &str,
query: &str,
response: Result<_Row, PgError>,
) -> i64 {
match response {
Ok(_) => {}
Ok(_) => 0,
Err(e) => {
// Permit runtime errors conservatively.
if let Some(e) = e.as_db_error()
&& is_permissible_error(e)
{
return;
return 1;
}
panic!(
"
Expand Down

0 comments on commit 4b55926

Please sign in to comment.