Disallow repeat_row with WITH ORDINALITY

Also disallow it in constructs that are implemented by WITH ORDINALITY
under the hood:
- ROWS FROM
- implicit ROWS FROM, i.e., multiple table functions in a SELECT clause

Author: ggevay

src/expr/src/lib.rs

@@ -33,6 +33,7 @@ pub use interpret::{ColumnSpec, ColumnSpecs, Interpreter, ResultSpec, Trace, Tra
 pub use linear::plan::{MfpPlan, SafeMfpPlan};
 pub use linear::util::{join_permutations, permutation_for_arrangement};
 pub use linear::{MapFilterProject, memoize_expr};
+pub use relation::func::REPEAT_ROW_NAME;
 pub use relation::func::order_aggregate_datums as order_aggregate_datums_exported_for_benchmarking;
 pub use relation::func::{
     AggregateFunc, AnalyzedRegex, AnalyzedRegexOpts, CaptureGroupDesc, LagLeadType,

src/expr/src/relation/func.rs

@@ -3496,7 +3496,6 @@ impl TableFunc {
             | TableFunc::GenerateSeriesTimestamp
             | TableFunc::GenerateSeriesTimestampTz
             | TableFunc::GuardSubquerySize { .. }
-            | TableFunc::RepeatRow // TODO: will move this to the not allowed category in the next commit
             | TableFunc::RepeatRowNonNegative
             | TableFunc::UnnestArray { .. }
             | TableFunc::UnnestList { .. }
@@ -3507,9 +3506,13 @@ impl TableFunc {
             | TableFunc::RegexpMatches => Some(TableFunc::WithOrdinality(WithOrdinality {
                 inner: Box::new(inner),
             })),
-            // IMPORTANT: Before adding a new table function here, consider negative diffs:
+            // IMPORTANT: Before adding a new table function above, consider negative diffs:
             // `WithOrdinality::eval` will panic if the inner table function emits a negative diff.
-            TableFunc::WithOrdinality(_) => None,
+            // (Note that negative diffs in the table function's _input_ don't matter. The table
+            // function implementation doesn't see the input diffs, so the thing that matters here
+            // is whether the table function itself can emit a negative diff.)
+            TableFunc::RepeatRow // can produce negative diffs
+            | TableFunc::WithOrdinality(_) => None, // no nesting of `WITH ORDINALITY` allowed
         }
     }
 }
@@ -3882,7 +3885,7 @@ impl fmt::Display for TableFunc {
             TableFunc::GenerateSeriesTimestampTz => f.write_str("generate_series"),
             TableFunc::GenerateSubscriptsArray => f.write_str("generate_subscripts"),
             TableFunc::GuardSubquerySize { .. } => f.write_str("guard_subquery_size"),
-            TableFunc::RepeatRow => f.write_str("repeat_row"),
+            TableFunc::RepeatRow => f.write_str(REPEAT_ROW_NAME),
             TableFunc::RepeatRowNonNegative => f.write_str("repeat_row_non_negative"),
             TableFunc::UnnestArray { .. } => f.write_str("unnest_array"),
             TableFunc::UnnestList { .. } => f.write_str("unnest_list"),
@@ -3947,3 +3950,5 @@ impl WithOrdinality {
         Ok(Box::new(it))
     }
 }
+
+pub const REPEAT_ROW_NAME: &str = "repeat_row";

src/sql/src/func.rs

@@ -4733,7 +4733,7 @@ pub static MZ_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
                 })
             }) => ReturnType::set_of(RecordAny), oid::FUNC_REGEXP_EXTRACT_OID;
         },
-        "repeat_row" => Table {
+        mz_expr::REPEAT_ROW_NAME => Table {
             params!(Int64) => Operation::unary(move |ecx, n| {
                 ecx.require_feature_flag(&vars::ENABLE_REPEAT_ROW)?;
                 Ok(TableFuncPlan {

src/sql/src/plan/query.rs

@@ -52,8 +52,8 @@ use mz_expr::func::variadic::{
 };
 use mz_expr::virtual_syntax::AlgExcept;
 use mz_expr::{
-    Id, LetRecLimit, LocalId, MapFilterProject, MirScalarExpr, RowSetFinishing, TableFunc,
-    func as expr_func,
+    Id, LetRecLimit, LocalId, MapFilterProject, MirScalarExpr, REPEAT_ROW_NAME, RowSetFinishing,
+    TableFunc, func as expr_func,
 };
 use mz_ore::assert_none;
 use mz_ore::collections::CollectionExt;
@@ -66,6 +66,7 @@ use mz_repr::adt::char::CharLength;
 use mz_repr::adt::numeric::{NUMERIC_DATUM_MAX_PRECISION, NumericMaxScale};
 use mz_repr::adt::timestamp::TimestampPrecision;
 use mz_repr::adt::varchar::VarCharMaxLength;
+use mz_repr::namespaces::MZ_CATALOG_SCHEMA;
 use mz_repr::{
     CatalogItemId, ColumnIndex, ColumnName, Datum, RelationDesc, RelationVersionSelector,
     ReprColumnType, Row, RowArena, SqlColumnType, SqlRelationType, SqlScalarType,
@@ -2826,6 +2827,14 @@ fn plan_scalar_table_funcs(
         }
         return Ok((expr, scope));
     }
+    if table_funcs.keys().any(is_repeat_row) {
+        // Note: Would be also caught by WITH ORDINALITY checking for `repeat_row`, but then the
+        // error message would be misleading, because it would refer to WITH ORDINALITY.
+        bail_unsupported!(format!(
+            "{} in a SELECT clause with multiple table functions",
+            REPEAT_ROW_NAME
+        ));
+    }
     // Otherwise, plan as usual, emulating the ROWS FROM behavior
     let (expr, mut scope, num_cols) =
         plan_rows_from_internal(&rows_from_qcx, table_funcs.keys(), None)?;
@@ -3111,6 +3120,14 @@ fn plan_rows_from(
     alias: Option<&TableAlias>,
     with_ordinality: bool,
 ) -> Result<(HirRelationExpr, Scope), PlanError> {
+    // The `repeat_row` function is not supported in ROWS FROM.
+    if functions.iter().any(is_repeat_row) {
+        // Note: Would be also caught by WITH ORDINALITY checking for `repeat_row`, but then the
+        // error message would be misleading, because it would refer to WITH ORDINALITY instead of
+        // ROWS FROM.
+        bail_unsupported!(format!("{} in ROWS FROM", REPEAT_ROW_NAME));
+    }
+
     // If there's only a single table function, planning proceeds as if `ROWS
     // FROM` hadn't been written at all.
     if let [function] = functions {
@@ -3155,6 +3172,10 @@ fn plan_rows_from(
     Ok((expr, scope))
 }
 
+fn is_repeat_row(f: &Function<Aug>) -> bool {
+    f.name.full_name_str().as_str() == format!("{}.{}", MZ_CATALOG_SCHEMA, REPEAT_ROW_NAME)
+}
+
 /// Plans an expression coalescing multiple table functions. Each table
 /// function is followed by its row ordinality. The entire expression is
 /// followed by the coalesced row ordinality.

test/sqllogictest/table_func.slt

@@ -477,15 +477,8 @@ SELECT * FROM generate_series(0,3), repeat_row_non_negative(generate_series);
 3
 3
 
-query II
+query error WITH ORDINALITY on repeat_row not yet supported
 SELECT * FROM generate_series(0,3), repeat_row(generate_series) WITH ORDINALITY;
-----
-1  1
-2  1
-2  2
-3  1
-3  2
-3  3
 
 query II
 SELECT * FROM generate_series(0,3), repeat_row_non_negative(generate_series) WITH ORDINALITY;
@@ -497,13 +490,8 @@ SELECT * FROM generate_series(0,3), repeat_row_non_negative(generate_series) WIT
 3  2
 3  3
 
-query II
+query error WITH ORDINALITY on repeat_row not yet supported
 SELECT * FROM generate_series(0,3), repeat_row(abs(generate_series - 1)) WITH ORDINALITY;
-----
-0  1
-2  1
-3  1
-3  2
 
 query II
 SELECT * FROM generate_series(0,3), repeat_row_non_negative(abs(generate_series - 1)) WITH ORDINALITY;
@@ -842,21 +830,11 @@ SELECT jsonb_array_elements.generate_series FROM ROWS FROM (jsonb_array_elements
 generate_series
 1
 
-query T colnames,rowsort
+query error repeat_row in ROWS FROM not yet supported
 SELECT repeat_row FROM ROWS FROM (repeat_row(2), generate_series(1, 1));
-----
-repeat_row
-()
-(1)
 
-query TI
+query error repeat_row in a SELECT clause with multiple table functions not yet supported
 SELECT repeat_row(5), generate_series(1,2);
-----
-()  NULL
-()  NULL
-()  NULL
-()  1
-()  2
 
 query TI
 SELECT repeat_row_non_negative(5), generate_series(1,2);