feat: add bounded Metric logical-table batch DDL (#9427)

* feat: add bounded Metric logical-table batch DDL Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: enforce logical batch DDL interceptor boundaries Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: bound rewritten DDL size through shared SQL formatting Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: preserve logical DDL error diagnostics Signed-off-by: jeremyhi <fengjiachun@gmail.com> * test: verify logical DDL error logging Signed-off-by: jeremyhi <fengjiachun@gmail.com> --------- Signed-off-by: jeremyhi <fengjiachun@gmail.com>
2026-10-20 10:52:32 +00:00 · 2026-10-08 06:33:56 +00:00
parent f9ab3c296b
commit c82da8308b
12 changed files with 1493 additions and 112 deletions
@@ -13,7 +13,7 @@
 // limitations under the License.

 use std::collections::HashMap;
-use std::fmt::{Display, Formatter};
+use std::fmt::{Display, Formatter, Write};

 use common_catalog::consts::FILE_ENGINE;
 use datatypes::json::{JSON2_DEFAULT_MAX_AUTO_EXPANDED_PATHS, JsonSettings};
@@ -68,6 +68,36 @@ fn format_table_constraint(constraints: &[TableConstraint]) -> String {
    constraints.iter().map(|c| format_indent!(c)).join(LINE_SEP)
 }

+fn write_list<T: Display>(
+    f: &mut dyn Write,
+    values: &[T],
+    separator: &str,
+    prefix: &str,
+) -> std::fmt::Result {
+    for (index, value) in values.iter().enumerate() {
+        if index > 0 {
+            f.write_str(separator)?;
+        }
+        write!(f, "{prefix}{value}")?;
+    }
+    Ok(())
+}
+
+// sqlparser's Custom Display joins all modifiers into an intermediate String.
+fn write_data_type(f: &mut dyn Write, data_type: &DataType) -> std::fmt::Result {
+    if let DataType::Custom(name, modifiers) = data_type {
+        write!(f, "{name}")?;
+        if !modifiers.is_empty() {
+            f.write_char('(')?;
+            write_list(f, modifiers, COMMA_SEP, "")?;
+            f.write_char(')')?;
+        }
+        Ok(())
+    } else {
+        write!(f, "{data_type}")
+    }
+}
+
 /// Table constraint for create table statement.
 #[derive(Debug, PartialEq, Eq, Clone, Visit, VisitMut, Serialize)]
 pub enum TableConstraint {
@@ -81,7 +111,9 @@ impl Display for TableConstraint {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        match self {
            TableConstraint::PrimaryKey { columns } => {
-                write!(f, "PRIMARY KEY ({})", format_list_comma!(columns))
+                f.write_str("PRIMARY KEY (")?;
+                write_list(f, columns, COMMA_SEP, "")?;
+                f.write_str(")")
            }
            TableConstraint::TimeIndex { column } => {
                write!(f, "TIME INDEX ({})", column)
@@ -143,12 +175,34 @@ pub struct Json2Options {

 impl Display for Json2Options {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        let mut options = Vec::with_capacity(self.type_hints.len() + 1);
+        f.write_str("(\n    ")?;
        if let Some(max) = self.max_auto_expanded_paths {
-            options.push(format!("max_auto_expanded_paths = {max}"));
+            write!(f, "max_auto_expanded_paths = {max}")?;
        }
-        options.extend(self.type_hints.iter().map(format_json_type_hint));
-        write!(f, "(\n    {}\n  )", options.iter().join(",\n    "))
+        for (index, hint) in self.type_hints.iter().enumerate() {
+            if index > 0 || self.max_auto_expanded_paths.is_some() {
+                f.write_str(",\n    ")?;
+            }
+            for (index, segment) in hint.path.iter().enumerate() {
+                if index > 0 {
+                    f.write_char('.')?;
+                }
+                f.write_char('"')?;
+                for part in segment.split_inclusive('"') {
+                    f.write_str(part)?;
+                    if part.ends_with('"') {
+                        f.write_char('"')?;
+                    }
+                }
+                f.write_char('"')?;
+            }
+            f.write_char(' ')?;
+            write_data_type(f, &hint.data_type)?;
+            if hint.inverted_index {
+                f.write_str(" INVERTED INDEX")?;
+            }
+        }
+        f.write_str("\n  )")
    }
 }

@@ -203,14 +257,20 @@ impl Column {

 impl Display for Column {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        if let Some(vector_options) = &self.extensions.vector_options
+        self.write_sql(f, false)
+    }
+}
+
+impl Column {
+    fn write_sql(&self, f: &mut dyn Write, complete: bool) -> std::fmt::Result {
+        if !complete
+            && let Some(vector_options) = &self.extensions.vector_options
            && let Some(dim) = vector_options.get(VECTOR_OPT_DIM)
        {
-            write!(f, "{} VECTOR({})", self.column_def.name, dim)?;
-            return Ok(());
+            return write!(f, "{} VECTOR({})", self.column_def.name, dim);
        }
-
-        write!(f, "{} {}", self.column_def.name, self.column_def.data_type)?;
+        write!(f, "{} ", self.column_def.name)?;
+        write_data_type(f, &self.column_def.data_type)?;
        if let Some(options) = &self.extensions.json2_options {
            write!(f, "{options}")?;
        }
@@ -218,30 +278,18 @@ impl Display for Column {
            write!(f, " {option}")?;
        }

-        if let Some(fulltext_options) = &self.extensions.fulltext_index_options {
-            if !fulltext_options.is_empty() {
-                let options = fulltext_options.kv_pairs();
-                write!(f, " FULLTEXT INDEX WITH({})", format_list_comma!(options))?;
-            } else {
-                write!(f, " FULLTEXT INDEX")?;
-            }
-        }
-
-        if let Some(skipping_index_options) = &self.extensions.skipping_index_options {
-            if !skipping_index_options.is_empty() {
-                let options = skipping_index_options.kv_pairs();
-                write!(f, " SKIPPING INDEX WITH({})", format_list_comma!(options))?;
-            } else {
-                write!(f, " SKIPPING INDEX")?;
-            }
-        }
-
-        if let Some(inverted_index_options) = &self.extensions.inverted_index_options {
-            if !inverted_index_options.is_empty() {
-                let options = inverted_index_options.kv_pairs();
-                write!(f, " INVERTED INDEX WITH({})", format_list_comma!(options))?;
-            } else {
-                write!(f, " INVERTED INDEX")?;
+        for (kind, options) in [
+            ("FULLTEXT", &self.extensions.fulltext_index_options),
+            ("SKIPPING", &self.extensions.skipping_index_options),
+            ("INVERTED", &self.extensions.inverted_index_options),
+        ] {
+            if let Some(options) = options {
+                write!(f, " {kind} INDEX")?;
+                if !options.is_empty() {
+                    f.write_str(" WITH(")?;
+                    options.write_sql(f, COMMA_SEP, "", complete)?;
+                    f.write_str(")")?;
+                }
            }
        }

@@ -317,24 +365,6 @@ fn json_type_hint_sql_data_type(data_type: &ConcreteDataType) -> Result<DataType
    Ok(sql_type)
 }

-fn format_json_type_hint(hint: &JsonTypeHint) -> String {
-    let path = hint
-        .path
-        .iter()
-        .map(|segment| format_json_path_segment(segment))
-        .join(".");
-    let inverted_index = if hint.inverted_index {
-        " INVERTED INDEX"
-    } else {
-        ""
-    };
-    format!("{} {}{}", path, hint.data_type, inverted_index)
-}
-
-fn format_json_path_segment(segment: &str) -> String {
-    format!("\"{}\"", segment.replace('"', "\"\""))
-}
-
 /// Partition on columns or values.
 ///
 /// - `column_list` is the list of columns in `PARTITION ON COLUMNS` clause.
@@ -359,12 +389,11 @@ impl Partitions {
 impl Display for Partitions {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        if !self.column_list.is_empty() {
-            write!(
-                f,
-                "PARTITION ON COLUMNS ({}) (\n{}\n)",
-                format_list_comma!(self.column_list),
-                format_list_indent!(self.exprs),
-            )?;
+            f.write_str("PARTITION ON COLUMNS (")?;
+            write_list(f, &self.column_list, COMMA_SEP, "")?;
+            f.write_str(") (\n")?;
+            write_list(f, &self.exprs, LINE_SEP, "  ")?;
+            f.write_str("\n)")?;
        }
        Ok(())
    }
@@ -372,6 +401,53 @@ impl Display for Partitions {

 impl Display for CreateTable {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        self.write_sql(f, false)
+    }
+}
+
+impl CreateTable {
+    /// Debits the SQL size of a rewritten AST, including unredacted and non-scalar
+    /// options. Returns false on exhaustion or a shape hidden by normal Display.
+    /// No SQL or secret value is returned. Delegated sqlparser formatters retain
+    /// their own allocation behavior; this is not a general AST memory limit.
+    pub fn consume_sql_size_budget(&self, remaining: &mut usize) -> bool {
+        struct Budget<'a>(&'a mut usize);
+        impl Write for Budget<'_> {
+            fn write_str(&mut self, value: &str) -> std::fmt::Result {
+                *self.0 = self.0.checked_sub(value.len()).ok_or(std::fmt::Error)?;
+                Ok(())
+            }
+        }
+        if self
+            .partitions
+            .as_ref()
+            .is_some_and(|p| p.column_list.is_empty() && !p.exprs.is_empty())
+        {
+            return false;
+        }
+        for column in &self.columns {
+            if let Some(options) = &column.extensions.vector_options {
+                let DataType::Custom(name, tokens) = &column.column_def.data_type else {
+                    return false;
+                };
+                if options.len() != 1
+                    || name.0.len() != 1
+                    || tokens.len() != 1
+                    || !name.0[0]
+                        .as_ident()
+                        .is_some_and(|name| name.value.eq_ignore_ascii_case("VECTOR"))
+                    || !tokens[0].parse::<u32>().ok().is_some_and(|dim| {
+                        options.get(VECTOR_OPT_DIM) == Some(dim.to_string().as_str())
+                    })
+                {
+                    return false;
+                }
+            }
+        }
+        self.write_sql(&mut Budget(remaining), true).is_ok()
+    }
+
+    fn write_sql(&self, f: &mut dyn Write, complete: bool) -> std::fmt::Result {
        write!(f, "CREATE ")?;
        if self.engine == FILE_ENGINE {
            write!(f, "EXTERNAL ")?;
@@ -381,16 +457,25 @@ impl Display for CreateTable {
            write!(f, "IF NOT EXISTS ")?;
        }
        writeln!(f, "{} (", &self.name)?;
-        writeln!(f, "{},", format_list_indent!(self.columns))?;
-        writeln!(f, "{}", format_table_constraint(&self.constraints))?;
+        for (index, column) in self.columns.iter().enumerate() {
+            if index > 0 {
+                f.write_str(LINE_SEP)?;
+            }
+            f.write_str("  ")?;
+            column.write_sql(f, complete)?;
+        }
+        f.write_str(",\n")?;
+        write_list(f, &self.constraints, LINE_SEP, "  ")?;
+        f.write_char('\n')?;
        writeln!(f, ")")?;
        if let Some(partitions) = &self.partitions {
            writeln!(f, "{partitions}")?;
        }
        writeln!(f, "ENGINE={}", &self.engine)?;
        if !self.options.is_empty() {
-            let options = self.options.kv_pairs();
-            write!(f, "WITH(\n{}\n)", format_list_indent!(options))?;
+            f.write_str("WITH(\n")?;
+            self.options.write_sql(f, LINE_SEP, "  ", complete)?;
+            f.write_str("\n)")?;
        }
        Ok(())
    }
@@ -623,6 +708,120 @@ mod tests {
    use crate::parser::{ParseOptions, ParserContext};
    use crate::statements::statement::Statement;

+    #[test]
+    fn test_rewritten_create_sql_budget() {
+        let parse = |sql| {
+            let mut statements = ParserContext::create_with_dialect(
+                sql,
+                &GreptimeDbDialect {},
+                ParseOptions::default(),
+            )
+            .unwrap();
+            let Statement::CreateTable(create) = statements.remove(0) else {
+                panic!("CREATE expected")
+            };
+            create
+        };
+        let create = parse(
+            "CREATE TABLE demo (ts TIMESTAMP TIME INDEX, host STRING) ENGINE=metric WITH (on_physical_table='phy')",
+        );
+        let size = create.to_string().len();
+        let mut remaining = size;
+        assert!(create.consume_sql_size_budget(&mut remaining));
+        assert_eq!(remaining, 0);
+        assert!(!create.consume_sql_size_budget(&mut (size - 1)));
+
+        for location in ["table", "fulltext", "skipping", "inverted"] {
+            for value_type in ["secret", "array", "struct"] {
+                for length in [10, 8192] {
+                    let mut create = create.clone();
+                    let extensions = &mut create.columns[1].extensions;
+                    let map = match location {
+                        "table" => &mut create.options,
+                        "fulltext" => extensions.fulltext_index_options.get_or_insert_default(),
+                        "skipping" => extensions.skipping_index_options.get_or_insert_default(),
+                        _ => extensions.inverted_index_options.get_or_insert_default(),
+                    };
+                    let value = "private-value".repeat(length);
+                    match value_type {
+                        "secret" => map.insert("secret_access_key".into(), value.clone()),
+                        "array" => map.insert_options("payload", vec![value.as_str()].into()),
+                        _ => map.insert_options(
+                            "payload",
+                            crate::util::OptionValue::try_new(Expr::Struct {
+                                values: vec![Expr::Value(
+                                    sqlparser::ast::Value::SingleQuotedString(value.clone()).into(),
+                                )],
+                                fields: vec![],
+                            })
+                            .unwrap(),
+                        ),
+                    }
+                    assert!(!create.to_string().contains(&value));
+                    assert_eq!(
+                        create.consume_sql_size_budget(&mut 4096),
+                        length == 10,
+                        "{location}/{value_type}"
+                    );
+                }
+            }
+        }
+        let mut vector = parse("CREATE TABLE demo (ts TIMESTAMP TIME INDEX, v VECTOR(3))");
+        vector.columns[1].column_def.options.push(ColumnOptionDef {
+            name: None,
+            option: sqlparser::ast::ColumnOption::Comment("private-value".repeat(8192)),
+        });
+        assert!(!vector.to_string().contains("private-value"));
+        assert!(!vector.consume_sql_size_budget(&mut 4096));
+        vector.columns[1].column_def.options.clear();
+        assert!(vector.consume_sql_size_budget(&mut 4096));
+        if let DataType::Custom(_, tokens) = &mut vector.columns[1].column_def.data_type {
+            tokens[0] = format!("{}3", "0".repeat(8192));
+        }
+        assert!(!vector.consume_sql_size_budget(&mut 4096));
+        vector.columns[1]
+            .extensions
+            .vector_options
+            .as_mut()
+            .unwrap()
+            .insert("extra".into(), "x".into());
+        assert!(!vector.consume_sql_size_budget(&mut 4096));
+
+        let mut create = create;
+        create.columns[1].extensions.json2_options = Some(Json2Options {
+            max_auto_expanded_paths: Some(1),
+            type_hints: vec![JsonTypeHint {
+                path: vec!["x".repeat(8192)],
+                data_type: DataType::Text,
+                inverted_index: true,
+            }],
+        });
+        assert!(!create.consume_sql_size_budget(&mut 4096));
+        create.columns[1].extensions.json2_options = None;
+        create.partitions = Some(Partitions {
+            column_list: vec![],
+            exprs: vec![Expr::Identifier(Ident::new("hidden"))],
+        });
+        assert!(!create.consume_sql_size_budget(&mut 4096));
+    }
+
+    #[test]
+    fn test_json2_streamed_quote_escaping() {
+        let path = "a\\\"\"b";
+        let options = Json2Options {
+            max_auto_expanded_paths: None,
+            type_hints: vec![JsonTypeHint {
+                path: vec![path.into()],
+                data_type: DataType::Text,
+                inverted_index: false,
+            }],
+        };
+        assert_eq!(
+            options.to_string(),
+            format!("(\n    \"{}\" TEXT\n  )", path.replace('"', "\"\""))
+        );
+    }
+
    #[test]
    fn test_display_create_table() {
        let sql = r"create table if not exists demo(
@@ -139,26 +139,44 @@ impl OptionMap {
    }

    pub fn kv_pairs(&self) -> Vec<String> {
-        let mut result = Vec::with_capacity(self.options.len() + self.secrets.len());
-        for (k, v) in self
-            .options
-            .iter()
-            .filter_map(|(k, v)| v.as_string().map(|v| (k, v)))
-        {
-            if k.contains(".") {
-                result.push(format!("'{k}' = '{}'", v.escape_debug()));
-            } else {
-                result.push(format!("{k} = '{}'", v.escape_debug()));
+        self.sql_pairs(false).map(|pair| pair.to_string()).collect()
+    }
+
+    /// Streams the same pairs as `kv_pairs`, or all actual values for size validation.
+    pub(crate) fn write_sql(
+        &self,
+        f: &mut dyn std::fmt::Write,
+        separator: &str,
+        prefix: &str,
+        complete: bool,
+    ) -> std::fmt::Result {
+        for (index, pair) in self.sql_pairs(complete).enumerate() {
+            if index > 0 {
+                f.write_str(separator)?;
            }
+            write!(f, "{prefix}{pair}")?;
        }
-        for (k, _) in self.secrets.iter() {
-            if k.contains(".") {
-                result.push(format!("'{k}' = '******'"));
+        Ok(())
+    }
+
+    fn sql_pairs(&self, complete: bool) -> impl Iterator<Item = SqlPair<'_>> {
+        let options = self.options.iter().filter_map(move |(key, value)| {
+            let value = match value.as_string() {
+                Some(value) => Either::Right(value),
+                None if complete => Either::Left(value),
+                None => return None,
+            };
+            Some(SqlPair { key, value })
+        });
+        let secrets = self.secrets.iter().map(move |(key, value)| SqlPair {
+            key,
+            value: Either::Right(if complete {
+                value.expose_secret()
            } else {
-                result.push(format!("{k} = '******'"));
-            }
-        }
-        result
+                "******"
+            }),
+        });
+        options.chain(secrets)
    }

    pub fn entries(&self) -> impl Iterator<Item = (&str, Either<&OptionValue, &str>)> {
@@ -174,6 +192,25 @@ impl OptionMap {
    }
 }

+struct SqlPair<'a> {
+    key: &'a str,
+    value: Either<&'a OptionValue, &'a str>,
+}
+
+impl std::fmt::Display for SqlPair<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        if self.key.contains('.') {
+            write!(f, "'{}'", self.key)?;
+        } else {
+            f.write_str(self.key)?;
+        }
+        match self.value {
+            Either::Left(value) => write!(f, " = {value}"),
+            Either::Right(value) => write!(f, " = '{}'", value.escape_debug()),
+        }
+    }
+}
+
 impl<I: IntoIterator<Item = (String, String)>> From<I> for OptionMap {
    fn from(value: I) -> Self {
        let mut result = OptionMap::default();
@@ -16,7 +16,6 @@ use std::collections::HashSet;
 use std::fmt::{Display, Formatter};
 use std::ops::ControlFlow;

-use itertools::Itertools;
 use promql_parser::label::{METRIC_NAME, MatchOp};
 use promql_parser::parser::{
    Expr as PromExpr, MatrixSelector as PromMatrixSelector, VectorSelector as PromVectorSelector,
@@ -161,12 +160,20 @@ impl Display for OptionValue {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        if let Some(s) = self.as_string() {
            write!(f, "'{s}'")
-        } else if let Some(s) = self.as_list() {
-            write!(
-                f,
-                "[{}]",
-                s.into_iter().map(|x| format!("'{x}'")).join(", ")
-            )
+        } else if let Expr::Array(array) = &self.0
+            && array
+                .elem
+                .iter()
+                .all(|expr| Self::expr_as_string(expr).is_some())
+        {
+            f.write_str("[")?;
+            for (index, expr) in array.elem.iter().enumerate() {
+                if index > 0 {
+                    f.write_str(", ")?;
+                }
+                write!(f, "'{}'", Self::expr_as_string(expr).unwrap())?;
+            }
+            f.write_str("]")
        } else {
            write!(f, "'{}'", self.0)
        }
@@ -667,6 +674,7 @@ fn extract_tables_from_sql_query(query: &sqlparser::ast::Query, names: &mut Hash

 #[cfg(test)]
 mod tests {
+    use itertools::Itertools;
    use sqlparser::tokenizer::Token;

    use super::*;