chore: Follow-up improvements to parquet subcommand CLI (#254)

Follow-up to #234. Updates docs, benchmarks, and CLI behavior for the
`parquet` subcommand.

## Changes

### CLI behavior
- Make `--format` optional (defaults to `tbl`) instead of requiring an
explicit value
- Warn when `--format=parquet` is used, directing users to the `parquet`
subcommand (v4.0.0 migration)
- Error when `--format` is used together with a subcommand
- Fix redundant "Warning:" prefix in `log::warn!` message

### Docs and benchmarks
- Update examples to use `parquet` subcommand instead of
`--format=parquet`
- Add deprecation notice to README for `--format=parquet`,
`--parquet-compression`, `--parquet-row-group-bytes`

### Tests
- Add integration tests for deprecation warning, format+subcommand
conflict, and default format

Author: kevinjqliu

benchmarks/BENCHMARKS.md

@@ -17,7 +17,7 @@ tpchgen-cli -s 100 --stdout | pv -arb > /dev/null
 # Outputs something similar to
 # 106GiB [3.09GiB/s] (3.09GiB/s)
 # For parquet
-tpchgen-cli -s 100 --format=parquet --stdout | pv -arb > /dev/null
+tpchgen-cli parquet -s 100 --stdout | pv -arb > /dev/null
 # 38.2GiB [ 865MiB/s] ( 865MiB/s)
 ```
 
@@ -49,7 +49,7 @@ single parquet file per table, with snappy page compression.
 Example command to create Scale Factor 10
 
 ```shell
-tpchgen-cli -s 10 --format=parquet
+tpchgen-cli parquet -s 10
 ```
 
 ## `parquet_duckdb.sh`

benchmarks/parquet_tpchgen.sh

@@ -13,5 +13,5 @@ uname -a >> $LOGFILE
 SCALE_FACTORS="1 10 100 1000"
 for sf in $SCALE_FACTORS ; do
     echo "SF=$sf" >> $LOGFILE
-    /usr/bin/time -a -o $LOGFILE tpchgen-cli -s $sf --output-dir=out_tpchgen --format=parquet
+    /usr/bin/time -a -o $LOGFILE tpchgen-cli parquet -s $sf --output-dir=out_tpchgen
 done

tpchgen-cli/README.md

@@ -45,16 +45,16 @@ RUSTFLAGS='-C target-cpu=native' cargo install tpchgen-cli
 ```shell
 # Scale Factor 10, all tables, in Apache Parquet format in the current directory
 # (3.6GB, 8 files, 60M lineitem rows, in 5 seconds on a modern laptop)
-tpchgen-cli -s 10 --format=parquet
+tpchgen-cli parquet -s 10
 
 # Scale Factor 10, all tables, in `tbl`(csv like) format in the `sf10` directory
 # (10GB, 8 files, 60M lineitem rows)
 tpchgen-cli -s 10 --output-dir sf10
 
 # Scale Factor 1000, lineitem table, in Apache Parquet format in sf1000 directory, 
-# 20 part(ititons), 100MB row groups
+# 20 part(itions), 100MB row groups
 # (220GB, 20 files, 6B lineitem rows, 3.5 minutes on a modern laptop)
-tpchgen-cli -s 1000 --tables lineitem --parts 20 --format=parquet --parquet-row-group-bytes=100000000 --output-dir sf1000
+tpchgen-cli parquet -s 1000 --tables lineitem --parts 20 --row-group-bytes=100000000 --output-dir sf1000
 
 # Scale Factor 10, partition 2 and 3 of 10 in sf10 directory
 #

tpchgen-cli/bin/main.rs

@@ -117,9 +117,12 @@ struct TopLevelArgs {
     #[command(flatten)]
     common: CommonArgs,
 
-    /// Output format: tbl, csv, parquet
-    #[arg(short, long, default_value = "tbl")]
-    format: OutputFormat,
+    /// Output format: tbl, csv, parquet (default: tbl)
+    ///
+    /// For Parquet output, prefer using the `parquet` subcommand instead.
+    /// The --format flag will be replaced by subcommands in v4.0.0.
+    #[arg(short, long)]
+    format: Option<OutputFormat>,
 
     /// Parquet block compression format (deprecated: use 'parquet' subcommand instead)
     #[arg(short = 'c', long, hide = true)]
@@ -251,6 +254,13 @@ async fn main() -> io::Result<()> {
 impl Cli {
     /// Main function to run the generation
     async fn main(self) -> io::Result<()> {
+        // Error if both --format and a subcommand are specified
+        if self.args.format.is_some() && self.command.is_some() {
+            return Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "Cannot use --format with a subcommand. Use the subcommand directly, e.g. `tpchgen-cli parquet`",
+            ));
+        }
         match self.command {
             Some(Commands::Parquet(args)) => args.run().await,
             None => self.run().await,
@@ -259,7 +269,7 @@ impl Cli {
 
     #[allow(deprecated)]
     async fn run(self) -> io::Result<()> {
-        let format = self.args.format;
+        let format = self.args.format.unwrap_or(OutputFormat::Tbl);
         let scale_factor = self.args.common.scale_factor;
         let output_dir = self.args.common.output_dir;
         let num_threads = self.args.common.num_threads;
@@ -275,10 +285,10 @@ impl Cli {
 
         configure_logging(verbose, quiet);
 
-        // Warn if parquet specific options are set but not generating parquet
+        // Warn about --format=parquet migration to subcommand
         if format == OutputFormat::Parquet {
             log::warn!(
-                "Warning: Use 'tpchgen-cli parquet' subcommand instead of '--format=parquet' for better validation and control"
+                "The --format=parquet flag will be replaced by the `parquet` subcommand in v4.0.0. Use `tpchgen-cli parquet` instead."
             );
         }
 
@@ -307,7 +317,7 @@ impl Cli {
 
         // Warn if delimiter is set but not generating CSV
         if format != OutputFormat::Csv && delimiter != ',' {
-            log::warn!("Warning: Delimiter option set but not generating CSV");
+            log::warn!("Delimiter option set but not generating CSV");
         }
 
         // Build the generator using the library API

tpchgen-cli/tests/cli_integration.rs

@@ -703,3 +703,68 @@ fn expect_row_group_sizes(output_dir: &Path, expected_row_groups: Vec<RowGroups>
     let actual_row_groups = format!("{actual_row_groups:#?}");
     assert_eq!(actual_row_groups, expected_row_groups);
 }
+
+/// Test that --format=parquet emits a warning about v4.0.0 migration
+#[tokio::test]
+async fn test_format_parquet_warns_about_subcommand() {
+    let output_dir = tempdir().unwrap();
+    cargo_bin_cmd!("tpchgen-cli")
+        .arg("--format")
+        .arg("parquet")
+        .arg("--tables")
+        .arg("part")
+        .arg("--scale-factor")
+        .arg("0.001")
+        .arg("--output-dir")
+        .arg(output_dir.path())
+        .assert()
+        .success()
+        .stderr(predicates::str::contains(
+            "will be replaced by the `parquet` subcommand in v4.0.0",
+        ));
+}
+
+/// Test that using --format together with a subcommand errors
+#[test]
+fn test_format_with_subcommand_conflict() {
+    let temp_dir = tempdir().expect("Failed to create temporary directory");
+
+    cargo_bin_cmd!("tpchgen-cli")
+        .arg("--format")
+        .arg("parquet")
+        .arg("parquet")
+        .arg("--scale-factor")
+        .arg("0.001")
+        .arg("--tables")
+        .arg("part")
+        .arg("--output-dir")
+        .arg(temp_dir.path())
+        .assert()
+        .failure()
+        .stderr(predicates::str::contains(
+            "Cannot use --format with a subcommand",
+        ));
+}
+
+/// Test that running with no --format and no subcommand defaults to TBL
+#[test]
+fn test_default_format_is_tbl() {
+    let temp_dir = tempdir().expect("Failed to create temporary directory");
+
+    cargo_bin_cmd!("tpchgen-cli")
+        .arg("--scale-factor")
+        .arg("0.001")
+        .arg("--tables")
+        .arg("part")
+        .arg("--output-dir")
+        .arg(temp_dir.path())
+        .assert()
+        .success();
+
+    let expected_file = temp_dir.path().join("part.tbl");
+    assert!(
+        expected_file.exists(),
+        "Expected TBL file {:?} to exist when no --format or subcommand is specified",
+        expected_file
+    );
+}