Rust PG server: fix-one batch snapshot of pg-batch (92aba52e, 3 commits)

changelog.d/rp-677-named-arg-unnamed-builtin.md

@@ -0,0 +1,14 @@
+### A named argument to a built-in without argument names is 42883
+
+A built-in such as `nextval` declares no argument names, so PostgreSQL finds
+no function for `nextval(regclass := 'seq')` and says so: 42883 `function
+nextval(regclass => unknown) does not exist`, at the call, with the
+no-function-matches hint. The Rust PostgreSQL server answered 0A000
+`NamedArgExpr is not supported yet` instead, a gap where PostgreSQL gives a
+definite answer. It now gives PostgreSQL's.
+
+#### Fixed
+
+- Rust PG server: a named argument to a built-in whose parameters have no
+  names is 42883 naming the call, each named argument printed `name => type`.
+  Fixes RP-677 (tests/backlog/test_rp_found_rp458.py::test_named_argument_to_a_builtin_without_argument_names)

changelog.d/rp-852-group-by-subquery-row.md

@@ -0,0 +1,9 @@
+### Grouping by a FROM subquery's row no longer groups its columns
+
+A query that groups by a FROM subquery's whole row, as in `select v.a from (values (1), (2)) v(a) group by v`, answered the rows as though `v.a` were grouped. PostgreSQL derives no functional dependency of a column on a subquery's row, so it rejects such a column outside an aggregate with 42803 at the reference, a select-list `v.*` included. The Rust PostgreSQL server now does the same, while a column grouped in its own right alongside the row is still accepted.
+
+#### Fixed
+
+- `GROUP BY v` over a FROM subquery: a column of `v` outside an aggregate and not itself grouped is 42803 `column "v.a" must appear in the GROUP BY clause or be used in an aggregate function`, positioned at the reference.
+
+Fixes RP-852 (tests/backlog/test_rp_found_rp699.py::test_group_by_a_from_subquery_row_leaves_its_columns_ungrouped)

changelog.d/rp767-setop-order-by-outer.md

@@ -0,0 +1,13 @@
+### A set operation's ORDER BY refuses an outer column as PostgreSQL does
+
+The ORDER BY of a UNION, INTERSECT or EXCEPT inside a correlated subquery
+sees only the set operation's output columns. A name that resolves to the
+outer row is a sort expression there, which PostgreSQL refuses as 0A000
+`invalid UNION/INTERSECT/EXCEPT ORDER BY clause`; the Rust PostgreSQL server
+now gives the same error, detail and position instead of 42703.
+
+#### Fixed
+
+- Rust PG server: `order by b` / `order by v.b` naming an outer column in a
+  correlated set operation is 0A000 with PostgreSQL's DETAIL, at the column.
+  Fixes RP-767 (tests/backlog/test_rp_found_rp698.py::test_an_outer_column_in_a_set_operations_order_by)

crates/secantus-pgplan/src/correlated.rs

@@ -843,6 +843,61 @@ fn fromless_outer_names(inner: &pg_query::protobuf::SelectStmt, outer: &[String]
     names
 }
 
+/// `transformSetOperationStmt`: a set operation's ORDER BY sees only its
+/// output columns, so a name resolving to the outer row is a sort
+/// expression, which it refuses (RP-767). A name neither level has stays
+/// 42703, found by planning.
+fn set_operation_order_by_outer(
+    inner: &pg_query::protobuf::SelectStmt,
+    outer: &[String],
+    outer_refs: &[Vec<String>],
+) -> Result<()> {
+    if inner.larg.is_none() {
+        return Ok(());
+    }
+    let mut leftmost = inner;
+    while let Some(l) = leftmost.larg.as_deref() {
+        leftmost = l;
+    }
+    let output: Vec<String> = leftmost
+        .target_list
+        .iter()
+        .filter_map(|t| match t.node.as_ref()? {
+            N::ResTarget(rt) if !rt.name.is_empty() => Some(rt.name.clone()),
+            N::ResTarget(rt) => match rt.val.as_ref()?.node.as_ref()? {
+                N::ColumnRef(c) => parts_of(c)?.pop(),
+                _ => None,
+            },
+            _ => None,
+        })
+        .collect();
+    for sb in &inner.sort_clause {
+        let Some(N::SortBy(sb)) = sb.node.as_ref() else {
+            continue;
+        };
+        let Some(N::ColumnRef(c)) = sb.node.as_deref().and_then(|n| n.node.as_ref()) else {
+            continue;
+        };
+        let Some(parts) = parts_of(c) else {
+            continue;
+        };
+        let is_outer = match parts.as_slice() {
+            [name] => !output.contains(name) && outer.contains(name),
+            _ => outer_refs.contains(&parts),
+        };
+        if is_outer {
+            crate::set_error_location(c.location);
+            return Err(Error::Sqlstate(
+                "0A000",
+                "invalid UNION/INTERSECT/EXCEPT ORDER BY clause\nDetail: Only result column \
+                 names can be used, not expressions or functions."
+                    .into(),
+            ));
+        }
+    }
+    Ok(())
+}
+
 pub(crate) fn correlate(
     sl: &pg_query::protobuf::SubLink,
     shape: &pg_query::protobuf::SubLink,
@@ -891,6 +946,7 @@ pub(crate) fn correlate(
         }
         Ok(())
     })?;
+    set_operation_order_by_outer(inner, outer, &outer_refs)?;
     // Unqualified ones, found by planning: a name the subquery cannot resolve
     // that the outer query has is a correlation; anything else is a typo.
     let first = params.len() + 1;

crates/secantus-pgplan/src/lib.rs

@@ -4121,21 +4121,66 @@ fn refuse_qualified_builtins(node: &N) -> Result<()> {
     Ok(())
 }
 
+/// A named argument matches only a parameter `pg_proc.proargnames` names
+/// (`FuncnameGetCandidates` / `MatchNamedCall`): a built-in with no
+/// argument names at all, `nextval` say, has no candidate for such a call,
+/// which is 42883 naming it with each named argument as `name => type`.
+fn refuse_named_args_to_unnamed_builtins(node: &N) -> Result<()> {
+    for (n, _, _, _) in node.nodes() {
+        let pg_query::NodeRef::FuncCall(f) = n else {
+            continue;
+        };
+        if !f
+            .args
+            .iter()
+            .any(|a| matches!(a.node.as_ref(), Some(N::NamedArgExpr(_))))
+        {
+            continue;
+        }
+        let Some(name) = func_name(f) else {
+            continue;
+        };
+        if f.funcname.len() > 1 && !func_schema_is_pg_catalog(f) {
+            continue;
+        }
+        if !funcsig::is_builtin_function_name(&name)
+            || named_args::has_argument_names(&name)
+            || correlated::user_function_named(&name)
+        {
+            continue;
+        }
+        return Err(no_such_call(f, &name));
+    }
+    Ok(())
+}
+
+fn func_schema_is_pg_catalog(f: &pg_query::protobuf::FuncCall) -> bool {
+    matches!(
+        f.funcname.first().and_then(|n| n.node.as_ref()),
+        Some(N::String(s)) if s.sval == "pg_catalog"
+    )
+}
+
 /// 42883 for a call PostgreSQL finds no function for, at the call, its
 /// arguments' types as it prints them (`unknown` for a bare literal).
 fn no_such_call(f: &pg_query::protobuf::FuncCall, written: &str) -> Error {
-    let types: Vec<String> = f
-        .args
-        .iter()
-        .map(|a| match a.node.as_ref() {
+    fn arg_type(a: &pg_query::protobuf::Node) -> String {
+        match a.node.as_ref() {
             Some(N::AConst(c))
                 if c.isnull || matches!(c.val, Some(pg_query::protobuf::a_const::Val::Sval(_))) =>
             {
                 "unknown".to_string()
             }
+            // `funcname_signature_string` prints a named argument as
+            // `name => type`.
+            Some(N::NamedArgExpr(na)) => match na.arg.as_deref() {
+                Some(v) => format!("{} => {}", na.name, arg_type(v)),
+                None => format!("{} => unknown", na.name),
+            },
             _ => display_type(&static_type(a, &Bson::Null)),
-        })
-        .collect();
+        }
+    }
+    let types: Vec<String> = f.args.iter().map(arg_type).collect();
     set_error_location(f.location);
     Error::UndefinedFunction(format!(
         "function {written}({}) does not exist",
@@ -6108,6 +6153,7 @@ fn plan_node(
     schemas::resolve_shadowed_calls(&mut node, lookup)?;
     collation::citext_operators(&mut node, lookup)?;
     refuse_qualified_builtins(&node)?;
+    refuse_named_args_to_unnamed_builtins(&node)?;
     refuse_invisible_extension_functions(&node)?;
     if let Some(st) = fdw::plan(&node, lookup, params)? {
         return Ok(st);
@@ -43894,6 +43940,137 @@ fn check_ungrouped_whole_row(
     }
 }
 
+/// A grouped query over a FROM subquery grouped by the subquery's row
+/// (`group by v`): its columns are not grouped by that, as PostgreSQL has
+/// no functional dependency of a column on a subquery's row
+/// (`check_ungrouped_columns`), so a column of it outside an aggregate --
+/// a select-list `v.*` included, at the star -- is 42803 (RP-852).
+fn check_columns_under_row_grouping(
+    s: &pg_query::protobuf::SelectStmt,
+    alias: &str,
+    cols: &[String],
+) -> Result<()> {
+    if cols.iter().any(|c| c == alias) {
+        return Ok(());
+    }
+    let names = |c: &pg_query::protobuf::ColumnRef| -> Vec<Option<String>> {
+        c.fields
+            .iter()
+            .map(|f| match f.node.as_ref() {
+                Some(N::String(sv)) => Some(sv.sval.clone()),
+                Some(N::AStar(_)) => Some("*".into()),
+                _ => None,
+            })
+            .collect()
+    };
+    let target_val = |i: usize| -> Option<&pg_query::protobuf::Node> {
+        match s.target_list.get(i)?.node.as_ref() {
+            Some(N::ResTarget(rt)) => rt.val.as_deref(),
+            _ => None,
+        }
+    };
+    let mut by_row = false;
+    let mut grouped: Vec<String> = Vec::new();
+    for g in &s.group_clause {
+        let g = match g.node.as_ref() {
+            Some(N::AConst(c)) => match &c.val {
+                Some(pg_query::protobuf::a_const::Val::Ival(i)) if i.ival >= 1 => {
+                    match target_val(i.ival as usize - 1) {
+                        Some(v) => v,
+                        None => continue,
+                    }
+                }
+                _ => continue,
+            },
+            _ => g,
+        };
+        let Some(N::ColumnRef(c)) = g.node.as_ref() else {
+            // Any other grouping item: leave the statement to the
+            // grouping check proper.
+            return Ok(());
+        };
+        match names(c).as_slice() {
+            [Some(a)] if a == alias => by_row = true,
+            [Some(a), Some(star)] if a == alias && star == "*" => by_row = true,
+            [Some(col)] | [Some(_), Some(col)] => grouped.push(col.clone()),
+            _ => return Ok(()),
+        }
+    }
+    if !by_row {
+        return Ok(());
+    }
+    let ungrouped = |col: &str| cols.iter().any(|c| c == col) && !grouped.iter().any(|g| g == col);
+    let mut found: Option<(i32, String)> = None;
+    let scan = |n: &pg_query::protobuf::Node, found: &mut Option<(i32, String)>| -> Result<()> {
+        let mut n = n.clone();
+        walk_expr(&mut n, &mut |x| {
+            let opaque = match x.node.as_ref() {
+                Some(N::SubLink(_)) | Some(N::GroupingFunc(_)) => true,
+                Some(N::FuncCall(f)) => {
+                    f.over.is_none()
+                        && func_name(f)
+                            .as_deref()
+                            .is_some_and(|n| aggregate_func(n, f.agg_within_group).is_some())
+                }
+                Some(N::ColumnRef(c)) if found.is_none() => {
+                    let col = match names(c).as_slice() {
+                        [Some(col)] => Some(col.clone()),
+                        [Some(a), Some(col)] if a == alias => Some(col.clone()),
+                        _ => None,
+                    };
+                    if let Some(col) = col.filter(|c| c != "*" && ungrouped(c)) {
+                        *found = Some((c.location, col));
+                    }
+                    false
+                }
+                _ => false,
+            };
+            if opaque {
+                *x = pg_query::protobuf::Node { node: None };
+            }
+            Ok(())
+        })
+    };
+    for t in &s.target_list {
+        let Some(N::ResTarget(rt)) = t.node.as_ref() else {
+            continue;
+        };
+        let Some(v) = rt.val.as_deref() else {
+            continue;
+        };
+        if let Some(N::ColumnRef(c)) = v.node.as_ref() {
+            let star = match names(c).as_slice() {
+                [Some(a), Some(st)] => a == alias && st == "*",
+                [Some(st)] => st == "*",
+                _ => false,
+            };
+            if star {
+                if found.is_none() {
+                    if let Some(col) = cols.iter().find(|c| ungrouped(c)) {
+                        found = Some((c.location, col.clone()));
+                    }
+                }
+                continue;
+            }
+        }
+        scan(v, &mut found)?;
+    }
+    if let Some(h) = s.having_clause.as_deref() {
+        scan(h, &mut found)?;
+    }
+    match found {
+        None => Ok(()),
+        Some((location, col)) => {
+            if location >= 0 {
+                set_error_location(location);
+            }
+            Err(Error::Grouping(format!(
+                "column \"{alias}.{col}\" must appear in the GROUP BY clause or be used in an aggregate function"
+            )))
+        }
+    }
+}
+
 /// Does the FROM function `rf` return one column? Planned for its output
 /// only, nothing in it called (as a subquery's names are, RP-472).
 fn function_returns_one_column(
@@ -44080,6 +44257,7 @@ fn whole_row_rewrite(
                     rowtype = Some(table_rowtype(table));
                 }
             }
+            check_columns_under_row_grouping(s, &alias, &cols)?;
             (alias, cols)
         }
         _ => return Ok(None),

crates/secantus-pgplan/src/named_args.rs

@@ -49,6 +49,11 @@ fn named(a: &pg_query::protobuf::Node) -> Option<&pg_query::protobuf::NamedArgEx
     }
 }
 
+/// Does some overload of this built-in name its arguments?
+pub(crate) fn has_argument_names(name: &str) -> bool {
+    signatures().contains_key(name)
+}
+
 /// Is this a call to a built-in with a named argument?
 pub(crate) fn wanted(f: &pg_query::protobuf::FuncCall) -> bool {
     f.args.iter().any(|a| named(a).is_some())