SLT: Print errors by default, introduce -q (MaterializeInc#35081)

You can either set -v or -q to make it less or more verbose.

I prefer the default of printing where an error happened, would have
saved me a bunch of rountrips of having to rerun with `-v`

Author: def-

src/sqllogictest/src/bin/sqllogictest.rs

@@ -32,13 +32,13 @@ use walkdir::WalkDir;
 /// Runs sqllogictest scripts to verify database engine correctness.
 #[derive(clap::Parser)]
 struct Args {
-    /// Increase verbosity.
-    ///
-    /// If specified once, print summary for each source file.
-    /// If specified twice, also show descriptions of each error.
-    /// If specified thrice, also print each query before it is executed.
-    #[clap(short = 'v', long = "verbose", action = ArgAction::Count)]
-    verbosity: u8,
+    /// By default, prints summary for each source file and shows descriptions of each error.
+    /// If specified, also print each query before it is executed.
+    #[clap(short = 'v', long = "verbose", action = ArgAction::SetTrue, conflicts_with = "quiet")]
+    verbose: bool,
+    /// Suppress summary output unless a file has failures.
+    #[clap(short = 'q', long = "quiet", action = ArgAction::SetTrue, conflicts_with = "verbose")]
+    quiet: bool,
     /// Don't exit with a failing code if not all queries are successful.
     #[clap(long)]
     no_fail: bool,
@@ -170,7 +170,8 @@ async fn main() -> ExitCode {
     let config = RunConfig {
         stdout: &OutputStream::new(io::stdout(), args.timestamps),
         stderr: &OutputStream::new(io::stderr(), args.timestamps),
-        verbosity: args.verbosity,
+        verbose: args.verbose,
+        quiet: args.quiet,
         postgres_url: args.postgres_url.clone(),
         prefix: args.prefix.clone(),
         no_fail: args.no_fail,
@@ -230,7 +231,7 @@ async fn main() -> ExitCode {
                     let start_time = Instant::now();
                     match runner::run_file(&mut runner, entry.path()).await {
                         Ok(o) => {
-                            if o.any_failed() || config.verbosity >= 1 {
+                            if o.any_failed() || !config.quiet {
                                 writeln!(
                                     config.stdout,
                                     "{}",

src/sqllogictest/src/runner.rs

@@ -443,7 +443,7 @@ pub struct RunnerInner<'a> {
     auto_index_selects: bool,
     auto_transactions: bool,
     enable_table_keys: bool,
-    verbosity: u8,
+    verbose: bool,
     stdout: &'a dyn WriteFmt,
     _shutdown_trigger: trigger::Trigger,
     _server_thread: JoinOnDropHandle<()>,
@@ -1307,7 +1307,7 @@ impl<'a> RunnerInner<'a> {
             auto_index_selects: config.auto_index_selects,
             auto_transactions: config.auto_transactions,
             enable_table_keys: config.enable_table_keys,
-            verbosity: config.verbosity,
+            verbose: config.verbose,
             stdout: config.stdout,
         };
         inner.ensure_fixed_features().await?;
@@ -1696,7 +1696,7 @@ impl<'a> RunnerInner<'a> {
         output: &'r Result<QueryOutput<'_>, &'r str>,
         location: Location,
     ) -> Result<Option<Outcome<'r>>, anyhow::Error> {
-        print_sql_if(self.stdout, sql, self.verbosity >= 2);
+        print_sql_if(self.stdout, sql, self.verbose);
         let sql_result = self.client.execute(sql, &[]).await;
 
         // Evaluate if we already reached an outcome or not.
@@ -1763,14 +1763,14 @@ impl<'a> RunnerInner<'a> {
         if let Some(outcome) = tentative_outcome {
             view_outcome = outcome;
         } else {
-            print_sql_if(self.stdout, view_sql.as_str(), self.verbosity >= 2);
+            print_sql_if(self.stdout, view_sql.as_str(), self.verbose);
             view_outcome = self
                 .execute_query(view_sql.as_str(), output, location.clone())
                 .await?;
         }
 
         // Remember to clean up after ourselves by dropping the view.
-        print_sql_if(self.stdout, drop_view.as_str(), self.verbosity >= 2);
+        print_sql_if(self.stdout, drop_view.as_str(), self.verbose);
         self.client.execute(drop_view.as_str(), &[]).await?;
 
         Ok(view_outcome)
@@ -1986,7 +1986,8 @@ pub trait WriteFmt {
 pub struct RunConfig<'a> {
     pub stdout: &'a dyn WriteFmt,
     pub stderr: &'a dyn WriteFmt,
-    pub verbosity: u8,
+    pub verbose: bool,
+    pub quiet: bool,
     pub postgres_url: String,
     pub prefix: String,
     pub no_fail: bool,
@@ -2120,10 +2121,10 @@ pub async fn run_string(
     writeln!(runner.config.stdout, "--- {}", source);
 
     for record in parser.parse_records()? {
-        // In maximal-verbosity mode, print the query before attempting to run
+        // In maximal-verbose mode, print the query before attempting to run
         // it. Running the query might panic, so it is important to print out
         // what query we are trying to run *before* we panic.
-        if runner.config.verbosity >= 2 {
+        if runner.config.verbose {
             print_record(runner.config, &record);
         }
 
@@ -2134,9 +2135,9 @@ pub async fn run_string(
             .unwrap();
 
         // Print warnings and failures in verbose mode.
-        if runner.config.verbosity >= 1 && !outcome.success() {
-            if runner.config.verbosity < 2 {
-                // If `verbosity >= 2`, we'll already have printed the record,
+        if !runner.config.quiet && !outcome.success() {
+            if !runner.config.verbose {
+                // If `verbose` is enabled, we'll already have printed the record,
                 // so don't print it again. Yes, this is an ugly bit of logic.
                 // Please don't try to consolidate it with the `print_record`
                 // call above, as it's important to have a mode in which records
@@ -2151,7 +2152,7 @@ pub async fn run_string(
                 }
                 print_record(runner.config, &record);
             }
-            if runner.config.verbosity >= 2 || outcome.failure() {
+            if runner.config.verbose || outcome.failure() {
                 writeln!(
                     runner.config.stdout,
                     "{}",

test/sqllogictest/mzcompose.py

@@ -238,7 +238,6 @@ def to_command(
         ]
         command = [
             "sqllogictest",
-            "-v",
             *([] if rewrite_results else self.flags),
             *sqllogictest_config,
             file,