fix: Make sure table name handling is done by the crate instead of clap (#92)

* fix: Make sure table name handling is done by the crate instead of clap

To allow supporting actual short hand we have to ensure that we use
a custom TypedValueParser that is able to parse the refs correctly
and align every abbrv with its long name and corresponding enum.

I don't like this code because it's a lot of gymnastics for supporting
command flags (I feel like clap is very much too powerful for our needs
but maybe I am used to Go's stdlib provided argument handler).

There's also some non-trivial complexity in the implementation (side
effect of how clap does things).

* fix: extend TypedValueParser with possible_values

Allows us to clarify the possible values for the tables argument.

* fix: remove hidden blocks since aliases are documented

Author: clflushopt

tpchgen-cli/src/main.rs

@@ -40,12 +40,14 @@ use crate::parquet::*;
 use crate::statistics::WriteStatistics;
 use crate::tbl::*;
 use ::parquet::basic::Compression;
+use clap::builder::TypedValueParser;
 use clap::{Parser, ValueEnum};
 use log::{debug, info, LevelFilter};
 use std::fmt::Display;
 use std::fs::{self, File};
 use std::io::{self, BufWriter, Stdout, Write};
 use std::path::PathBuf;
+use std::str::FromStr;
 use std::time::Instant;
 use tpchgen::distribution::Distributions;
 use tpchgen::generators::{
@@ -71,7 +73,7 @@ struct Cli {
     output_dir: PathBuf,
 
     /// Which tables to generate (default: all)
-    #[arg(short = 'T', long = "tables", value_enum)]
+    #[arg(short = 'T', long = "tables", value_delimiter = ',', value_parser = TableValueParser)]
     tables: Option<Vec<Table>>,
 
     /// Number of parts to generate (manual parallel generation)
@@ -115,7 +117,7 @@ struct Cli {
     stdout: bool,
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
 enum Table {
     Nation,
     Region,
@@ -133,6 +135,69 @@ impl Display for Table {
     }
 }
 
+#[derive(Debug, Clone)]
+struct TableValueParser;
+
+impl TypedValueParser for TableValueParser {
+    type Value = Table;
+
+    /// Parse the value into a Table enum.
+    fn parse_ref(
+        &self,
+        cmd: &clap::Command,
+        _: Option<&clap::Arg>,
+        value: &std::ffi::OsStr,
+    ) -> Result<Self::Value, clap::Error> {
+        let value = value
+            .to_str()
+            .ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidValue).with_cmd(cmd))?;
+        Table::from_str(value)
+            .map_err(|_| clap::Error::new(clap::error::ErrorKind::InvalidValue).with_cmd(cmd))
+    }
+
+    fn possible_values(
+        &self,
+    ) -> Option<Box<dyn Iterator<Item = clap::builder::PossibleValue> + '_>> {
+        Some(Box::new(
+            [
+                clap::builder::PossibleValue::new("Region").help("Region table (alias: r)"),
+                clap::builder::PossibleValue::new("Nation").help("Nation table (alias: n)"),
+                clap::builder::PossibleValue::new("Supplier").help("Supplier table (alias: s)"),
+                clap::builder::PossibleValue::new("Customer").help("Customer table (alias: c)"),
+                clap::builder::PossibleValue::new("Part").help("Part table (alias: P)"),
+                clap::builder::PossibleValue::new("PartSupp").help("PartSupp table (alias: S)"),
+                clap::builder::PossibleValue::new("Orders").help("Orders table (alias: O)"),
+                clap::builder::PossibleValue::new("LineItem").help("LineItem table (alias: L)"),
+            ]
+            .into_iter(),
+        ))
+    }
+}
+
+impl FromStr for Table {
+    type Err = &'static str;
+
+    /// Returns the table enum value from the given string full name or abbreviation
+    ///
+    /// The original dbgen tool allows some abbreviations to mean two different tables
+    /// like 'p' which aliases to both 'part' and 'partsupp'. This implementation does
+    /// not support this since it just adds unnecessary complexity and confusion so we
+    /// only support the exclusive abbreviations.
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "n" | "nation" => Ok(Table::Nation),
+            "r" | "region" => Ok(Table::Region),
+            "s" | "supplier" => Ok(Table::Supplier),
+            "P" | "part" => Ok(Table::Part),
+            "S" | "partsupp" => Ok(Table::Partsupp),
+            "c" | "customer" => Ok(Table::Customer),
+            "O" | "orders" => Ok(Table::Orders),
+            "L" | "lineitem" => Ok(Table::Lineitem),
+            _ => Err("Invalid table name {s}"),
+        }
+    }
+}
+
 impl Table {
     fn name(&self) -> &'static str {
         match self {