Skip to content

Commit 147685e

Browse files
SangJunBakclaude
andcommitted
expr: test parse_catalog_create_sql
mz_tables and mz_views select rows by parse_catalog_create_sql(...)->>'type' and read 'definition' and 'source_id' out of the same call, but the function had no tests. Pins the reported type for every statement kind an Item record can hold, the exact mz_views.definition rendering (which pg_views exposes and which moved here out of the deleted pack_view_update), its idempotence under re-parsing, source_id presence for CREATE TABLE FROM SOURCE and absence for plain and webhook tables, and the four error paths. The error paths matter more than they used to: the MVs call this inside their WHERE clause, so an item the parser rejects makes the whole relation unreadable rather than breaking one row's packing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent e9012e1 commit 147685e

1 file changed

Lines changed: 237 additions & 0 deletions

File tree

‎src/expr/src/scalar/func/impls/jsonb.rs‎

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1711,4 +1711,241 @@ mod tests {
17111711
let out = super::parse_catalog_create_sql(sql).expect("ok");
17121712
assert_eq!(as_serde(out).get("envelope_type"), None);
17131713
}
1714+
1715+
// --- parse_catalog_create_sql --------------------------------------------
1716+
1717+
/// `type` for a `create_sql`, or the error message if parsing failed.
1718+
fn item_type(sql: &str) -> Result<String, String> {
1719+
match super::parse_catalog_create_sql(sql) {
1720+
Ok(out) => match as_serde(out) {
1721+
serde_json::Value::Object(mut m) => match m.remove("type") {
1722+
Some(serde_json::Value::String(s)) => Ok(s),
1723+
other => panic!("no string `type` key: {other:?}"),
1724+
},
1725+
other => panic!("not a JSON object: {other:?}"),
1726+
},
1727+
Err(EvalError::InvalidCatalogJson(msg)) => Err(msg.to_string()),
1728+
Err(e) => panic!("unexpected error variant: {e:?}"),
1729+
}
1730+
}
1731+
1732+
fn view_sql(query: &str) -> String {
1733+
format!("CREATE VIEW \"materialize\".\"public\".\"v\" AS {query}")
1734+
}
1735+
1736+
/// `definition` for a `CREATE VIEW` whose query is `query`.
1737+
fn view_definition(query: &str) -> String {
1738+
match as_serde(super::parse_catalog_create_sql(&view_sql(query)).expect("ok")) {
1739+
serde_json::Value::Object(mut m) => match m.remove("definition") {
1740+
Some(serde_json::Value::String(s)) => s,
1741+
other => panic!("no string `definition` key: {other:?}"),
1742+
},
1743+
other => panic!("not a JSON object: {other:?}"),
1744+
}
1745+
}
1746+
1747+
/// `mz_tables` and `mz_views` select rows by
1748+
/// `parse_catalog_create_sql(...)->>'type'`, and the function runs over
1749+
/// every `Item` row in the catalog, so a statement kind that changes its
1750+
/// reported type silently gains or loses rows in those relations. Pin the
1751+
/// type of every kind the catalog can hold.
1752+
#[mz_ore::test]
1753+
#[cfg_attr(miri, ignore)] // error: unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux`
1754+
fn catalog_item_type_per_statement_kind() {
1755+
let cases = [
1756+
(
1757+
"CREATE TABLE \"materialize\".\"public\".\"t\" (a int4)",
1758+
"table",
1759+
),
1760+
// A table created from a source, and a webhook table, are both
1761+
// `table`, so both land in mz_tables.
1762+
(
1763+
"CREATE TABLE \"materialize\".\"public\".\"tbl\" \
1764+
FROM SOURCE [u1 AS \"materialize\".\"public\".\"src\"] \
1765+
(REFERENCE = \"topic\") FORMAT TEXT",
1766+
"table",
1767+
),
1768+
(
1769+
"CREATE TABLE \"materialize\".\"public\".\"wht\" FROM WEBHOOK BODY FORMAT JSON",
1770+
"table",
1771+
),
1772+
(
1773+
"CREATE VIEW \"materialize\".\"public\".\"v\" AS SELECT 1",
1774+
"view",
1775+
),
1776+
(
1777+
"CREATE MATERIALIZED VIEW \"materialize\".\"public\".\"mv\" \
1778+
IN CLUSTER [u1] AS SELECT 1",
1779+
"materialized-view",
1780+
),
1781+
(
1782+
"CREATE SOURCE \"materialize\".\"public\".\"lg\" \
1783+
IN CLUSTER [u1] FROM LOAD GENERATOR COUNTER",
1784+
"source",
1785+
),
1786+
(
1787+
"CREATE SOURCE \"materialize\".\"public\".\"wh\" \
1788+
IN CLUSTER [u1] FROM WEBHOOK BODY FORMAT JSON",
1789+
"source",
1790+
),
1791+
(
1792+
"CREATE SUBSOURCE \"materialize\".\"public\".\"sub\" (id int4) \
1793+
OF SOURCE [u1 AS \"materialize\".\"public\".\"src\"]",
1794+
"subsource",
1795+
),
1796+
(
1797+
"CREATE SUBSOURCE \"materialize\".\"public\".\"progress\" (id int4) \
1798+
WITH (PROGRESS)",
1799+
"subsource",
1800+
),
1801+
(
1802+
"CREATE SINK \"materialize\".\"public\".\"snk\" IN CLUSTER [u1] \
1803+
FROM [u1 AS \"materialize\".\"public\".\"t\"] \
1804+
INTO KAFKA CONNECTION [u2 AS \"materialize\".\"public\".\"c\"] \
1805+
(TOPIC 'tp') FORMAT JSON ENVELOPE DEBEZIUM",
1806+
"sink",
1807+
),
1808+
(
1809+
"CREATE INDEX \"i\" IN CLUSTER [u1] \
1810+
ON [u1 AS \"materialize\".\"public\".\"t\"] (\"a\")",
1811+
"index",
1812+
),
1813+
(
1814+
"CREATE TYPE \"materialize\".\"public\".\"ty\" AS LIST (ELEMENT TYPE = int4)",
1815+
"type",
1816+
),
1817+
(
1818+
"CREATE SECRET \"materialize\".\"public\".\"s\" AS 'x'",
1819+
"secret",
1820+
),
1821+
(
1822+
"CREATE CONNECTION \"materialize\".\"public\".\"c\" \
1823+
TO KAFKA (BROKER 'b', SECURITY PROTOCOL PLAINTEXT)",
1824+
"connection",
1825+
),
1826+
];
1827+
for (sql, expected) in cases {
1828+
assert_eq!(item_type(sql).as_deref(), Ok(expected), "for {sql}");
1829+
}
1830+
}
1831+
1832+
/// `mz_views.definition` is produced here. It used to be produced by
1833+
/// `pack_view_update` in the adapter, so the exact rendering is a
1834+
/// compatibility surface: `pg_views.definition` reads it.
1835+
#[mz_ore::test]
1836+
#[cfg_attr(miri, ignore)] // error: unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux`
1837+
fn catalog_view_definition() {
1838+
// Identifiers and function names come back fully quoted, literals
1839+
// untouched, and PostgreSQL's trailing semicolon is appended.
1840+
assert_eq!(view_definition("SELECT 1"), "SELECT 1;");
1841+
assert_eq!(
1842+
view_definition("WITH c AS (SELECT 1 AS a) SELECT a FROM c"),
1843+
"WITH \"c\" AS (SELECT 1 AS \"a\") SELECT \"a\" FROM \"c\";"
1844+
);
1845+
assert_eq!(
1846+
view_definition("SELECT 1 UNION ALL SELECT 2"),
1847+
"SELECT 1 UNION ALL SELECT 2;"
1848+
);
1849+
assert_eq!(
1850+
view_definition("SELECT (SELECT max(a) FROM [u1 AS \"materialize\".\"public\".\"t\"])"),
1851+
"SELECT (SELECT \"max\"(\"a\") FROM [u1 AS \"materialize\".\"public\".\"t\"]);"
1852+
);
1853+
// Identifiers needing quotes, an embedded double quote, non-ASCII, an
1854+
// embedded single quote in a literal, and ORDER BY all survive.
1855+
assert_eq!(
1856+
view_definition(
1857+
"SELECT \"a b\", \"héllo\", \"q\"\"x\" \
1858+
FROM [u1 AS \"materialize\".\"public\".\"t\"] \
1859+
WHERE s = 'lit''eral' AND n = 42 ORDER BY 1"
1860+
),
1861+
"SELECT \"a b\", \"héllo\", \"q\"\"x\" \
1862+
FROM [u1 AS \"materialize\".\"public\".\"t\"] \
1863+
WHERE \"s\" = 'lit''eral' AND \"n\" = 42 ORDER BY 1;"
1864+
);
1865+
}
1866+
1867+
/// The rendering must be a fixed point: `pg_views` consumers re-issue
1868+
/// `definition` as the body of a new view, so a second pass through the
1869+
/// parser has to produce the identical string. The trailing `;` is part of
1870+
/// what gets re-parsed.
1871+
#[mz_ore::test]
1872+
#[cfg_attr(miri, ignore)] // error: unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux`
1873+
fn catalog_view_definition_is_idempotent() {
1874+
for query in [
1875+
"SELECT 1",
1876+
"WITH c AS (SELECT 1 AS a) SELECT a FROM c",
1877+
"SELECT 1 UNION ALL SELECT 2",
1878+
"SELECT \"a b\", \"q\"\"x\" FROM [u1 AS \"materialize\".\"public\".\"t\"] \
1879+
WHERE s = 'lit''eral' ORDER BY 1",
1880+
] {
1881+
let once = view_definition(query);
1882+
assert_eq!(
1883+
view_definition(&once),
1884+
once,
1885+
"not a fixed point for {query}"
1886+
);
1887+
}
1888+
}
1889+
1890+
/// `mz_tables.source_id` comes from this key. A table with no source must
1891+
/// omit it entirely, so the MV's `->>'source_id'` yields SQL NULL.
1892+
#[mz_ore::test]
1893+
#[cfg_attr(miri, ignore)] // error: unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux`
1894+
fn catalog_table_source_id() {
1895+
let from_source = as_serde(
1896+
super::parse_catalog_create_sql(
1897+
"CREATE TABLE \"materialize\".\"public\".\"tbl\" \
1898+
FROM SOURCE [u1 AS \"materialize\".\"public\".\"src\"] \
1899+
(REFERENCE = \"topic\") FORMAT TEXT",
1900+
)
1901+
.expect("ok"),
1902+
);
1903+
assert_eq!(from_source, json!({ "type": "table", "source_id": "u1" }));
1904+
1905+
for sql in [
1906+
"CREATE TABLE \"materialize\".\"public\".\"t\" (a int4)",
1907+
"CREATE TABLE \"materialize\".\"public\".\"wht\" FROM WEBHOOK BODY FORMAT JSON",
1908+
] {
1909+
assert_eq!(
1910+
as_serde(super::parse_catalog_create_sql(sql).expect("ok")),
1911+
json!({ "type": "table" }),
1912+
"for {sql}"
1913+
);
1914+
}
1915+
}
1916+
1917+
/// Every error here is fatal to the whole of `mz_tables`/`mz_views`, not to
1918+
/// one row: the MVs call this function inside their `WHERE` clause, so an
1919+
/// item the parser rejects makes the relation unreadable for everyone.
1920+
#[mz_ore::test]
1921+
#[cfg_attr(miri, ignore)] // error: unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux`
1922+
fn catalog_create_sql_errors() {
1923+
assert_eq!(
1924+
item_type("this is not sql"),
1925+
Err(
1926+
"failed to parse create_sql: Expected a keyword at the beginning of a statement, \
1927+
found identifier \"this\""
1928+
.to_string()
1929+
)
1930+
);
1931+
assert_eq!(
1932+
item_type("CREATE TABLE t (a int4); CREATE TABLE u (b int4)"),
1933+
Err("expected a single statement, found 2".to_string())
1934+
);
1935+
// A statement that is not a CREATE of a catalog item, e.g. if a future
1936+
// change persists something else in an Item record.
1937+
assert_eq!(
1938+
item_type("SELECT 1"),
1939+
Err("not a CREATE item statement".to_string())
1940+
);
1941+
// Catalog `create_sql` always names items by id. An unresolved name
1942+
// means the record was written wrong.
1943+
assert_eq!(
1944+
item_type(
1945+
"CREATE TABLE \"materialize\".\"public\".\"tbl\" \
1946+
FROM SOURCE src (REFERENCE = \"topic\")"
1947+
),
1948+
Err("unresolved item name".to_string())
1949+
);
1950+
}
17141951
}

0 commit comments

Comments
 (0)