diff --git a/.circleci/run_tests.sh b/.circleci/run_tests.sh index 4ba497c3..3e076c06 100644 --- a/.circleci/run_tests.sh +++ b/.circleci/run_tests.sh @@ -10,7 +10,7 @@ set -o xtrace # for inspection. function start_pgcat() { kill -s SIGINT $(pgrep pgcat) || true - RUST_LOG=${1} ./target/debug/pgcat .circleci/pgcat.toml & + ./target/debug/pgcat .circleci/pgcat.toml --log-level ${1} & sleep 1 } @@ -29,6 +29,9 @@ PGPASSWORD=sharding_user pgbench -h 127.0.0.1 -U sharding_user shard2 -i LOG_LEVEL=error toxiproxy-server & sleep 1 +# Remove toxiproxy if it exists. +toxiproxy-cli delete postgres_replica || true + # Create a database at port 5433, forward it to Postgres toxiproxy-cli create -l 127.0.0.1:5433 -u 127.0.0.1:5432 postgres_replica diff --git a/src/client.rs b/src/client.rs index 98a0669c..4989f8e3 100644 --- a/src/client.rs +++ b/src/client.rs @@ -890,7 +890,7 @@ where 'Q' => { if query_router.query_parser_enabled() { - match query_router.parse(&message) { + initial_parsed_ast = Some(match query_router.parse(&message) { Ok(ast) => { let plugin_result = query_router.execute_plugins(&ast).await; @@ -910,15 +910,20 @@ where let _ = query_router.infer(&ast); - initial_parsed_ast = Some(ast); + ast } Err(error) => { - warn!( - "Query parsing error: {} (client: {})", - error, client_identifier - ); + if error != Error::UnsupportedStatement { + warn!( + "Query parsing error: {} (client: {})", + error, client_identifier + ); + } + // Returning an empty vector shows the failed parse attempt and + // avoids another round of parse attempt below. + vec![] } - } + }) } } @@ -940,10 +945,12 @@ where let _ = query_router.infer(&ast); } Err(error) => { - warn!( - "Query parsing error: {} (client: {})", - error, client_identifier - ); + if error != Error::UnsupportedStatement { + warn!( + "Query parsing error: {} (client: {})", + error, client_identifier + ); + } } }; } @@ -1273,14 +1280,25 @@ where if query_router.query_parser_enabled() { // We don't want to parse again if we already parsed it as the initial message let ast = match initial_parsed_ast { - Some(_) => Some(initial_parsed_ast.take().unwrap()), + Some(_) => { + let parsed_ast = initial_parsed_ast.take().unwrap(); + // if 'parsed_ast' is empty, it means that there was a failed + // attempt to parse the query as a custom command, earlier above. + if parsed_ast.is_empty() { + None + } else { + Some(parsed_ast) + } + } None => match query_router.parse(&message) { Ok(ast) => Some(ast), Err(error) => { - warn!( - "Query parsing error: {} (client: {})", - error, client_identifier - ); + if error != Error::UnsupportedStatement { + warn!( + "Query parsing error: {} (client: {})", + error, client_identifier + ); + } None } }, diff --git a/src/query_router.rs b/src/query_router.rs index 8b451dd3..4b6873dc 100644 --- a/src/query_router.rs +++ b/src/query_router.rs @@ -338,6 +338,9 @@ impl QueryRouter { Some((command, value)) } + const UNSUPPORTED_STATEMENTS_FOR_PARSING: [&'static str; 4] = + ["COPY", "SET", "TRUNCATE", "VACUUM"]; + pub fn parse(&self, message: &BytesMut) -> Result, Error> { let mut message_cursor = Cursor::new(message); @@ -380,8 +383,22 @@ impl QueryRouter { match Parser::parse_sql(&PostgreSqlDialect {}, &query) { Ok(ast) => Ok(ast), Err(err) => { - debug!("{}: {}", err, query); - Err(Error::QueryRouterParserError(err.to_string())) + let qry_upper = query.to_ascii_uppercase(); + + // Check for unsupported statements to avoid producing a warning. + // Note 1: this is not a complete list of unsupported statements. + // Note 2: we do not check for unsupported statements before going through the + // parser, as the plugin system might be able to handle them, once sqlparser + // is able to correctly parse these (rather valid) queries. + if Self::UNSUPPORTED_STATEMENTS_FOR_PARSING + .iter() + .any(|s| qry_upper.starts_with(s)) + { + Err(Error::UnsupportedStatement) + } else { + debug!("{}: {}", err, query); + Err(Error::QueryRouterParserError(err.to_string())) + } } } } diff --git a/tests/docker/run.sh b/tests/docker/run.sh index ae30c978..bfffc570 100644 --- a/tests/docker/run.sh +++ b/tests/docker/run.sh @@ -1,6 +1,18 @@ #!/bin/bash -rm -rf /app/target/ || true +set -e + +CLEAN_BUILD=true + +if [ $1 = "no-clean" ]; then + echo "INFO: clean build is NOT going to be performed." + CLEAN_BUILD=false + find /app/target/debug/deps -name *.gcda -exec rm {} \; +fi + +if $CLEAN_BUILD ; then + rm -rf /app/target/ || true +fi rm /app/*.profraw || true rm /app/pgcat.profdata || true rm -rf /app/cov || true @@ -12,7 +24,9 @@ export RUSTFLAGS="-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Co export RUSTDOCFLAGS="-Cpanic=abort" cd /app/ -cargo clean +if $CLEAN_BUILD ; then + cargo clean +fi cargo build cargo test --tests diff --git a/tests/pgbench/simple.sql b/tests/pgbench/simple.sql index ad5e6139..8a01cb4a 100644 --- a/tests/pgbench/simple.sql +++ b/tests/pgbench/simple.sql @@ -26,7 +26,7 @@ UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid; INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP); -END; +COMMIT; SET SHARDING KEY TO :aid;