Repository: risingwavelabs/risingwave (language: python)

Issue:
bug(frontend): pgwire binary response unimplemented for list and struct
### Describe the bug

pgwire binary response unimplemented for list and struct

### To Reproduce

```python
import psycopg

conn = psycopg.connect(host='localhost', port='4566', dbname='dev', user='root')
with conn.cursor() as cur:
	cur.execute("select Array[1::bigint, 2::bigint, 3::bigint]", binary=True)
	for row in cur:
		print(row)
```

### Expected behavior

No panic in frontend.

### Additional context

_No response_
Sqlsmith: Sql feature generation
As we add more features, these should be tested via sqlsmith as well.

SQL Features:
- [ ] Generate other table functions: `generate_series`, `unnest`. See: https://github.com/singularity-data/risingwave/issues/4540
- [ ] Generate array_agg, jsonb_agg, jsonb_object_agg
- [ ] Generate select from System Tables
- [ ] Generate `NATURAL JOIN`
- [ ] Generate `JOIN` with `USING`
- [ ] Generate query w/o `GROUP BY`
- [ ] Randomly toggle session variables
- [ ] Generate `pow`/`exp`.
- [ ] Generate TopN by group: https://www.risingwave.dev/docs/current/sql-pattern-topn/
- [ ] Generate jsonb exprs, see: https://github.com/risingwavelabs/risingwave/issues/7714
- [ ] #7132
- [x] Generate order-by-clauses with `NULLS { FIRST | LAST }` (will be done in https://github.com/risingwavelabs/risingwave/pull/10864/commits/000cbc11cbc04f20abfd127b1bf4b3fd24e75c44)
- [ ] Generate `max`,`min`,`count` for `timestamptz`
- [ ] Generate `bit_and`, `bit_or` and `big_xor` in batch queries, and `bit_xor` in streaming queries.
- [ ] Generate array range access.
- [ ] https://github.com/risingwavelabs/risingwave/issues/10241
- [ ] Generate window functions other than `row_number`, `rank`.
- [ ] Generate `ALTER` DDL statements.\
- [ ] https://github.com/risingwavelabs/risingwave/issues/10516
- [ ] Generate temporal filter in sqlsmith


Discussion / hints:
Loading binary for array here: https://github.com/psycopg/psycopg/blob/0d2ea7648336712988206ac1dac28f01be775413/psycopg/psycopg/types/array.py#L434
https://github.com/risingwavelabs/risingwave/issues/3676#issuecomment-1254826683

@ZENOTME Do we have plan to support them?
It's not easy to support multiple demision list so there is no work for that now.

https://github.com/MaterializeInc/materialize/blob/61a82dfd8432b094189c4f839f67ab738f0f3bc1/src/pgrepr/src/value.rs#L390-L421

We can try to implement a single dimension list first. BTW, rust-postgres seems also only support to single dimension list. 
We should avoid its panic first 😄
> single dimension list.

Let's support the single-dim array first. I think it's doable. At least we can eliminate the majority of error cases.
+1 for https://github.com/risingwavelabs/risingwave/issues/7949#issuecomment-1978126393 & https://github.com/risingwavelabs/risingwave/issues/7949#issuecomment-1432819897. Let's finish these first.


Fix patch:
diff --git a/src/common/src/types/jsonb.rs b/src/common/src/types/jsonb.rs
index 9e6fdf8641cb7..4b25741fbe96a 100644
--- a/src/common/src/types/jsonb.rs
+++ b/src/common/src/types/jsonb.rs
@@ -133,8 +133,8 @@ impl crate::types::to_binary::ToBinary for JsonbRef<'_> {
     fn to_binary_with_type(
         &self,
         _ty: &crate::types::DataType,
-    ) -> super::to_binary::Result<Option<bytes::Bytes>> {
-        Ok(Some(self.value_serialize().into()))
+    ) -> super::to_binary::Result<bytes::Bytes> {
+        Ok(self.value_serialize().into())
     }
 }
 
diff --git a/src/common/src/types/mod.rs b/src/common/src/types/mod.rs
index 44be87116643c..ad516eab101c6 100644
--- a/src/common/src/types/mod.rs
+++ b/src/common/src/types/mod.rs
@@ -987,7 +987,7 @@ pub fn hash_datum(datum: impl ToDatumRef, state: &mut impl std::hash::Hasher) {
 impl ScalarRefImpl<'_> {
     pub fn binary_format(&self, data_type: &DataType) -> to_binary::Result<Bytes> {
         use self::to_binary::ToBinary;
-        self.to_binary_with_type(data_type).transpose().unwrap()
+        self.to_binary_with_type(data_type)
     }
 
     pub fn text_format(&self, data_type: &DataType) -> String {
diff --git a/src/common/src/types/num256.rs b/src/common/src/types/num256.rs
index 6c96b3ddbbec8..eccb0a9741ea3 100644
--- a/src/common/src/types/num256.rs
+++ b/src/common/src/types/num256.rs
@@ -165,14 +165,11 @@ macro_rules! impl_common_for_num256 {
         }
 
         impl ToBinary for $scalar_ref<'_> {
-            fn to_binary_with_type(
-                &self,
-                _ty: &DataType,
-            ) -> super::to_binary::Result<Option<Bytes>> {
+            fn to_binary_with_type(&self, _ty: &DataType) -> super::to_binary::Result<Bytes> {
                 let mut output = bytes::BytesMut::new();
                 let buffer = self.to_be_bytes();
                 output.put_slice(&buffer);
-                Ok(Some(output.freeze()))
+                Ok(output.freeze())
             }
         }
 
diff --git a/src/common/src/types/to_binary.rs b/src/common/src/types/to_binary.rs
index da7f75f0a2a3f..7c5e88dbc10cf 100644
--- a/src/common/src/types/to_binary.rs
+++ b/src/common/src/types/to_binary.rs
@@ -12,13 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use bytes::{Bytes, BytesMut};
+use bytes::{BufMut, Bytes, BytesMut};
 use postgres_types::{ToSql, Type};
 
 use super::{
     DataType, Date, Decimal, Interval, ScalarRefImpl, Serial, Time, Timestamp, Timestamptz, F32,
     F64,
 };
+use crate::array::ListRef;
 use crate::error::NotImplemented;
 
 /// Error type for [`ToBinary`] trait.
@@ -38,19 +39,19 @@ pub type Result<T> = std::result::Result<T, ToBinaryError>;
 /// [`postgres_types::ToSql`] has similar functionality, and most of our types implement
 /// that trait and forward `ToBinary` to it directly.
 pub trait ToBinary {
-    fn to_binary_with_type(&self, ty: &DataType) -> Result<Option<Bytes>>;
+    fn to_binary_with_type(&self, ty: &DataType) -> Result<Bytes>;
 }
 macro_rules! implement_using_to_sql {
     ($({ $scalar_type:ty, $data_type:ident, $accessor:expr } ),* $(,)?) => {
         $(
             impl ToBinary for $scalar_type {
-                fn to_binary_with_type(&self, ty: &DataType) -> Result<Option<Bytes>> {
+                fn to_binary_with_type(&self, ty: &DataType) -> Result<Bytes> {
                     match ty {
                         DataType::$data_type => {
                             let mut output = BytesMut::new();
                             #[allow(clippy::redundant_closure_call)]
                             $accessor(self).to_sql(&Type::ANY, &mut output).map_err(ToBinaryError::ToSql)?;
-                            Ok(Some(output.freeze()))
+                            Ok(output.freeze())
                         },
                         _ => unreachable!(),
                     }
@@ -78,8 +79,45 @@ implement_using_to_sql! {
     { Timestamptz, Timestamptz, |x: &Timestamptz| x.to_datetime_utc() }
 }
 
+impl ToBinary for ListRef<'_> {
+    fn to_binary_with_type(&self, ty: &DataType) -> Result<Bytes> {
+        // Reference: Postgres code `src/backend/utils/adt/arrayfuncs.c`
+        // https://github.com/postgres/postgres/blob/c1c09007e219ae68d1f8428a54baf68ccc1f8683/src/backend/utils/adt/arrayfuncs.c#L1548
+        use crate::row::Row;
+        let element_ty = match ty {
+            DataType::List(ty) => ty.as_ref(),
+            _ => unreachable!(),
+        };
+        if matches!(element_ty, DataType::List(_)) {
+            bail_not_implemented!(
+                issue = 7949,
+                "list with 2 or more dimensions is not supported"
+            )
+        }
+        let mut buf = BytesMut::new();
+        buf.put_i32(1); // Number of dimensions (must be 1)
+        buf.put_i32(1); // Has nulls?
+        buf.put_i32(element_ty.to_oid()); // Element type
+        buf.put_i32(self.len() as i32); // Length of 1st dimension
+        buf.put_i32(0); // Offset of 1st dimension
+        for element in self.iter() {
+            match element {
+                None => {
+                    buf.put_i32(-1); // -1 length means a NULL
+                }
+                Some(value) => {
+                    let data = value.to_binary_with_type(element_ty)?;
+                    buf.put_i32(data.len() as i32); // Length of element
+                    buf.put(data);
+                }
+            }
+        }
+        Ok(buf.into())
+    }
+}
+
 impl ToBinary for ScalarRefImpl<'_> {
-    fn to_binary_with_type(&self, ty: &DataType) -> Result<Option<Bytes>> {
+    fn to_binary_with_type(&self, ty: &DataType) -> Result<Bytes> {
         match self {
             ScalarRefImpl::Int16(v) => v.to_binary_with_type(ty),
             ScalarRefImpl::Int32(v) => v.to_binary_with_type(ty),
@@ -98,11 +136,13 @@ impl ToBinary for ScalarRefImpl<'_> {
             ScalarRefImpl::Time(v) => v.to_binary_with_type(ty),
             ScalarRefImpl::Bytea(v) => v.to_binary_with_type(ty),
             ScalarRefImpl::Jsonb(v) => v.to_binary_with_type(ty),
-            ScalarRefImpl::Struct(_) | ScalarRefImpl::List(_) => bail_not_implemented!(
-                issue = 7949,
-                "the pgwire extended-mode encoding for {ty} is unsupported"
-            ),
-            ScalarRefImpl::Map(_) => todo!(),
+            ScalarRefImpl::List(v) => v.to_binary_with_type(ty),
+            ScalarRefImpl::Struct(_) | ScalarRefImpl::Map(_) => {
+                bail_not_implemented!(
+                    issue = 7949,
+                    "the pgwire extended-mode encoding for {ty} is unsupported"
+                )
+            }
         }
     }
 }


Test patch:
diff --git a/e2e_test/extended_mode/1dim_list.slt.part b/e2e_test/extended_mode/1dim_list.slt.part
new file mode 100644
index 0000000000000..1e067bd6bfebd
--- /dev/null
+++ b/e2e_test/extended_mode/1dim_list.slt.part
@@ -0,0 +1,57 @@
+# Test binary format of 1-dimension lists (arrays)
+
+query T
+select ARRAY['foo', 'bar', null];
+----
+{foo,bar,NULL}
+
+query T
+select ARRAY[1,2+3,4*5+1];
+----
+{1,5,21}
+
+query T
+select ARRAY[null];
+----
+{NULL}
+
+statement error
+select ARRAY[];
+
+query T
+select ARRAY[]::int[];
+----
+{}
+
+statement ok
+create table t (v1 int);
+
+statement ok
+insert into t values (1), (2), (3);
+
+query T rowsort
+select ARRAY[1, v1*2] from t;
+----
+{1,2}
+{1,4}
+{1,6}
+
+query T
+select min(ARRAY[1, v1*2]) from t;
+----
+{1,2}
+
+query T
+select max(ARRAY[1, v1*2]) from t;
+----
+{1,6}
+
+query T
+select array[false, false] from t;
+----
+{f,f}
+{f,f}
+{f,f}
+
+statement ok
+drop table t;
\ No newline at end of file
diff --git a/e2e_test/extended_mode/README.md b/e2e_test/extended_mode/README.md
new file mode 100644
index 0000000000000..2c96d7e0c2326
--- /dev/null
+++ b/e2e_test/extended_mode/README.md
@@ -0,0 +1,5 @@
+# How to run
+
+```shell
+sqllogictest -p 4566 -d dev -e postgres-extended './e2e_test/extended_mode/**/*.slt'
+```
diff --git a/e2e_test/extended_mode/type.slt b/e2e_test/extended_mode/type.slt
index b172fcf389abc..c6e5c51b5c048 100644
--- a/e2e_test/extended_mode/type.slt
+++ b/e2e_test/extended_mode/type.slt
@@ -3,10 +3,10 @@
 statement ok
 SET RW_IMPLICIT_FLUSH TO true;
 
-# RisingWave can't support list and struct now so we skip them.
-# include ../batch/types/array.slt.part
-# include ../batch/types/struct.slt.part
-# include ../batch/types/list.slt.part
+include 1dim_list.slt.part
+
+# RisingWave can't support struct now so we skip it.
+# include ../batch/types/struct/*.slt.part
 
 # Sqllogitest can't support binary format bytea type so we skip it.
 # include ../batch/types/bytea.slt.part
diff --git a/src/tests/e2e_extended_mode/README.md b/src/tests/e2e_extended_mode/README.md
index 07e67d68e3a0c..53e44e6ac8e44 100644
--- a/src/tests/e2e_extended_mode/README.md
+++ b/src/tests/e2e_extended_mode/README.md
@@ -1,16 +1,16 @@
 This is a program used for e2e test in extended mode.
 
-## What is difference between it and extended_mode/*.slt in e2e_test
+## What is difference between this and `e2e_test/extended_mode`
+
+For e2e test in extended query mode, there are a few things we can't test in sqllogictest
 
-For e2e test in extended query mode, there are two thing we can't test in sqllogitest
 1. bind parameter
 2. max row number
 3. cancel query
-See [detail](https://www.postgresql.org/docs/15/protocol-flow.html#PROTOCOL-FLOW-PIPELINING:~:text=Once%20a%20portal,count%20is%20ignored)
 
-So before sqllogictest supporting these, we test these function in this program.
+See more details [here](https://www.postgresql.org/docs/15/protocol-flow.html#PROTOCOL-FLOW-PIPELINING:~:text=Once%20a%20portal,count%20is%20ignored).
 
-In the future, we may merge it to e2e_text/extended_query
+Before sqllogictest supports these, we test these functions in this program. In the future, we may merge it to `e2e_test/extended_mode`.
 
 # How to run