Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 34 additions & 9 deletions src/expr/src/scalar/func/variadic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use md5::Md5;
use mz_lowertest::MzReflect;
use mz_ore::cast::{CastFrom, ReinterpretCast};
use mz_pgtz::timezone::TimezoneSpec;
use mz_repr::adt::array::ArrayDimension;
use mz_repr::adt::array::{ArrayDimension, ArrayDimensions, InvalidArrayError};
use mz_repr::adt::mz_acl_item::{AclItem, AclMode, MzAclItem};
use mz_repr::adt::range::{InvalidRangeError, Range, RangeBound, parse_range_bound_flags};
use mz_repr::adt::system::Oid;
Expand Down Expand Up @@ -77,19 +77,44 @@ pub fn and<'a>(
/// the SQL type system, rather than checked here at runtime.)
///
/// If all input arrays are zero-dimensional arrays, then the output is a zero-
/// dimensional array. Otherwise the lower bound of the additional dimension is
/// dimensional array. Otherwise, the lower bound of the additional dimension is
/// one and the length of the new dimension is equal to `datums.len()`.
///
/// Null elements are allowed and considered to be zero-dimensional arrays.
fn array_create_multidim<'a>(
datums: &[Datum<'a>],
temp_storage: &'a RowArena,
) -> Result<Datum<'a>, EvalError> {
// Per PostgreSQL, if all input arrays are zero dimensional, so is the
// output.
if datums.iter().all(|d| d.unwrap_array().dims().is_empty()) {
let dims = &[];
let datums = &[];
let datum = temp_storage.try_make_datum(|packer| packer.try_push_array(dims, datums))?;
return Ok(datum);
let mut dim: Option<ArrayDimensions> = None;
for datum in datums {
let actual_dims = match datum {
Datum::Null => ArrayDimensions::default(),
Datum::Array(arr) => arr.dims(),
d => panic!("unexpected datum {d}"),
};
if let Some(expected) = &dim {
if actual_dims.ndims() != expected.ndims() {
let actual = actual_dims.ndims().into();
let expected = expected.ndims().into();
// All input arrays must have the same dimensionality.
return Err(InvalidArrayError::WrongCardinality { actual, expected }.into());
}
if let Some((e, a)) = expected
.into_iter()
.zip_eq(actual_dims.into_iter())
.find(|(e, a)| e != a)
{
let actual = a.length;
let expected = e.length;
// All input arrays must have the same dimensionality.
return Err(InvalidArrayError::WrongCardinality { actual, expected }.into());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need another error variant to be precise, but I feel we're good with the current one being used in two different contexts (length vs. dims).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postgres also checks for the lower bound being the same, even if the lengths are the same and it also produces a more generic error message than us:

postgres=# SELECT ARRAY['[0:0]={1}'::int[], '[1:1]={2}'::int[]];
ERROR:  multidimensional arrays must have array expressions with matching dimensions

What do you think of changing our error variant to give the same generic error message and then here we just do a simple if actual_dims != expected_dims { ... } ?

}
}
dim = Some(actual_dims);
}
// Per PostgreSQL, if all input arrays are zero dimensional, so is the output.
if dim.as_ref().map_or(true, ArrayDimensions::is_empty) {
return Ok(temp_storage.try_make_datum(|packer| packer.try_push_array(&[], &[]))?);
}

let mut dims = vec![ArrayDimension {
Expand Down
6 changes: 6 additions & 0 deletions src/repr/src/adt/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ pub struct ArrayDimensions<'a> {
pub(crate) data: &'a [u8],
}

impl Default for ArrayDimensions<'static> {
fn default() -> Self {
Self { data: &[] }
}
}

impl ArrayDimensions<'_> {
/// Returns the number of dimensions in the array as a [`u8`].
pub fn ndims(&self) -> u8 {
Expand Down
13 changes: 13 additions & 0 deletions test/sqllogictest/arrays.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1460,3 +1460,16 @@ statement ok
CREATE MATERIALIZED VIEW json_mv AS (
SELECT * FROM jsons WHERE random_id = CAST(payload->>'my_field' AS uuid[])[random_index]
)

# Regression test for issue MaterializeInc/database-issues#9757

query error db error: ERROR: number of array elements \(1\) does not match declared cardinality \(0\)
SELECT ARRAY[NULL::BIGINT[], ARRAY[1::BIGINT, 2::BIGINT]];

query T
SELECT ARRAY[NULL::BIGINT[], ARRAY[]::BIGINT[], NULL::BIGINT[]];
----
{}

query error db error: ERROR: number of array elements \(3\) does not match declared cardinality \(2\)
SELECT ARRAY[ARRAY[1, 2], ARRAY[3, 4, 5], ARRAY[6]];