risinglightdb / sqllogictest-rs

Sqllogictest (dialect with extensions) parser and runner in Rust.
Apache License 2.0
172 stars 45 forks source link

Proposal: Add support for checking for stream errors #171

Closed jon-chuang closed 9 months ago

jon-chuang commented 1 year ago

Hello, we are proposing to integrate stream error checking into sqllogictest. The purpose is to query a background datasource for stream errors recorded during a given period.

The interface is as follows (from: https://github.com/risingwavelabs/risingwave/issues/8037#issuecomment-1436250050):

The interface for StreamErrorAdapter:

trait StreamErrorAdapter {
  fn add_data_sources(endpoints: &[str]);
  fn get_current_count(error_pattern: &str) -> u64;
  // Internally, can implement retry until satisfied unless timed out logic
  fn ensure_current_count(error_pattern: &str, count: u64, timeout_ms: u64) -> Result<()>;
}

The current syntax for pg response errors are:

statement error QueryError: sql parser error: Expected identifier.*

The syntax for stream error reporting will be:

statement stream error (count 10) QueryError: sql parser error: Expected identifier.*

The implementation looks like:

// Initialize sqllogictest with data sources
sqllogictest ... --stream_error_data_sources="192.168.1.1:8099,192.168.2.1:8099"

in *.slt:

statement stream error (count 10) QueryError: sql parser error: Expected identifier.*
create materialized view v as ... 

statement stream error (count 10) QueryError: sql parser error: Expected identifier.*
insert into t1 values(...) // This causes parse error

In sqllogictest-rs:

let before = adapter.get_current_count("QueryError: sql parser error: Expected identifier.*").await;
run_query(query).await?;
run_query("flush;").await?;
// The timeout is only dependent on some delays by e.g. prometheus in-memory collection, but not by risingwave dataflow.
adapter.ensure_current_count("QueryError: sql parser error: Expected identifier.*", before + 10, 100).await?;
skyzh commented 1 year ago

Actually I don't think this is part of the SQL standard... This is probably better done with sqllogictest extension and you might need to ship your custom runner.

ref https://github.com/risinglightdb/sqllogictest-rs/issues/95

jon-chuang commented 1 year ago

might need to ship your custom runner.

Hmm, does this mean fork?

sqllogictest extension

I guess not implemented yet?

Perhaps it's too much effort, we may just write bash scripts since our current surface area is small (and must test source connectors too that can't use slt).