risingwave: pg_wire: executing queries with field descriptor using pg protocol cause panic

Describe the bug

Queries that use pg protocol and involve any field descriptor (e.g. using PostgreSQL client of different languages) will panic the entire RisingWave frontend.

cursor.execute("select (2, '%s');", ("2333333",))

# or any other type
cursor.execute("select (2, %s);", (2333333,))

This bug is discovered because in certain circumstances, my RisingWave receives such a query from unknown sources:

Since for now RisingWave only supports very limited types of field descriptors, we may need to work around to trace the error to certain position. When I managed to trace it, it originates from pg_wire. The token that exactly causes this error (in the above query) seems to be "(2 "

thread 'tokio-runtime-worker' panicked at 'called `Option::unwrap()` on a `None` value', /Users/kiven/risingwave/src/utils/pgwire/src/pg_protocol.rs:332:55
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/panicking.rs:48:5
   3: core::option::Option<T>::unwrap
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/option.rs:775:21
   4: pgwire::pg_protocol::PgProtocol<S,SM>::process_parse_msg::{{closure}}::{{closure}}
             at ./src/utils/pgwire/src/pg_protocol.rs:332:35
   5: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &mut F>::call_once
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/ops/function.rs:301:13
   6: core::option::Option<T>::map
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/option.rs:929:29
   7: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::next
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/iter/adapters/map.rs:103:9
   8: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::next
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/iter/adapters/map.rs:103:9
   9: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/alloc/src/vec/spec_from_iter_nested.rs:26:32
  10: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/alloc/src/vec/spec_from_iter.rs:33:9
  11: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/alloc/src/vec/mod.rs:2648:9
  12: core::iter::traits::iterator::Iterator::collect
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/iter/traits/iterator.rs:1836:9
  13: pgwire::pg_protocol::PgProtocol<S,SM>::process_parse_msg::{{closure}}
             at ./src/utils/pgwire/src/pg_protocol.rs:326:17
  14: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/future/mod.rs:91:19
  15: pgwire::pg_protocol::PgProtocol<S,SM>::do_process_inner::{{closure}}
             at ./src/utils/pgwire/src/pg_protocol.rs:168:61
  16: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/future/mod.rs:91:19
  17: pgwire::pg_protocol::PgProtocol<S,SM>::do_process::{{closure}}
             at ./src/utils/pgwire/src/pg_protocol.rs:104:38
  18: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/future/mod.rs:91:19
  19: pgwire::pg_protocol::PgProtocol<S,SM>::process::{{closure}}
             at ./src/utils/pgwire/src/pg_protocol.rs:100:26
  20: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/future/mod.rs:91:19
  21: pgwire::pg_server::pg_serve::{{closure}}::{{closure}}
             at ./src/utils/pgwire/src/pg_server.rs:94:46
  22: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/future/mod.rs:91:19
  23: <tracing::instrument::Instrumented<T> as core::future::future::Future>::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tracing-0.1.35/src/instrument.rs:272:9
  24: tokio::runtime::task::core::CoreStage<T>::poll::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/core.rs:165:17
  25: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/loom/std/unsafe_cell.rs:14:9
  26: tokio::runtime::task::core::CoreStage<T>::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/core.rs:155:13
  27: tokio::runtime::task::harness::poll_future::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:480:19
  28: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/panic/unwind_safe.rs:271:9
  29: std::panicking::try::do_call
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/panicking.rs:492:40
  30: ___rust_try
  31: std::panicking::try
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/panicking.rs:456:19
  32: std::panic::catch_unwind
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/panic.rs:137:14
  33: tokio::runtime::task::harness::poll_future
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:468:18
  34: tokio::runtime::task::harness::Harness<T,S>::poll_inner
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:104:27
  35: tokio::runtime::task::harness::Harness<T,S>::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:57:15
  36: tokio::runtime::task::raw::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/raw.rs:144:5
  37: tokio::runtime::task::raw::RawTask::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/raw.rs:84:18
  38: tokio::runtime::task::LocalNotified<S>::run
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/mod.rs:381:9
  39: tokio::runtime::thread_pool::worker::Context::run_task::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/thread_pool/worker.rs:435:13
  40: tokio::coop::with_budget::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/coop.rs:102:9
  41: std::thread::local::LocalKey<T>::try_with
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/thread/local.rs:445:16
  42: std::thread::local::LocalKey<T>::with
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/thread/local.rs:421:9
  43: tokio::coop::with_budget
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/coop.rs:95:5
  44: tokio::coop::budget
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/coop.rs:72:5
  45: tokio::runtime::thread_pool::worker::Context::run_task
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/thread_pool/worker.rs:434:9
  46: tokio::runtime::thread_pool::worker::Context::run
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/thread_pool/worker.rs:401:24
  47: tokio::runtime::thread_pool::worker::run::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/thread_pool/worker.rs:386:17
  48: tokio::macros::scoped_tls::ScopedKey<T>::set
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/macros/scoped_tls.rs:61:9
  49: tokio::runtime::thread_pool::worker::run
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/thread_pool/worker.rs:383:5
  50: tokio::runtime::thread_pool::worker::Launch::launch::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/thread_pool/worker.rs:362:45
  51: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/blocking/task.rs:42:21
  52: <tracing::instrument::Instrumented<T> as core::future::future::Future>::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tracing-0.1.35/src/instrument.rs:272:9
  53: tokio::runtime::task::core::CoreStage<T>::poll::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/core.rs:165:17
  54: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/loom/std/unsafe_cell.rs:14:9
  55: tokio::runtime::task::core::CoreStage<T>::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/core.rs:155:13
  56: tokio::runtime::task::harness::poll_future::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:480:19
  57: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/core/src/panic/unwind_safe.rs:271:9
  58: std::panicking::try::do_call
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/panicking.rs:492:40
  59: ___rust_try
  60: std::panicking::try
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/panicking.rs:456:19
  61: std::panic::catch_unwind
             at /rustc/9067d5277d10f0f32a49ec9c125a33828e26a32b/library/std/src/panic.rs:137:14
  62: tokio::runtime::task::harness::poll_future
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:468:18
  63: tokio::runtime::task::harness::Harness<T,S>::poll_inner
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:104:27
  64: tokio::runtime::task::harness::Harness<T,S>::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/harness.rs:57:15
  65: tokio::runtime::task::raw::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/raw.rs:144:5
  66: tokio::runtime::task::raw::RawTask::poll
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/raw.rs:84:18
  67: tokio::runtime::task::UnownedTask<S>::run
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/task/mod.rs:418:9
  68: tokio::runtime::blocking::pool::Task::run
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/blocking/pool.rs:91:9
  69: tokio::runtime::blocking::pool::Inner::run
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/blocking/pool.rs:325:17
  70: tokio::runtime::blocking::pool::Spawner::spawn_thread::{{closure}}
             at /Users/kiven/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.1/src/runtime/blocking/pool.rs:300:13
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Thu Aug 25 11:23:14 UTC 2022 [risedev]: Program exited with 134

To Reproduce

Install psycopg package and its binary dependency, a PostgreSQL client in Python.

pip3 install psycopg psycopg_c  

Start either a minimum or a full cluster.

risedev d
risedev d full

Execute the following command as Python script, or in Python console:

import psycopg

conn = psycopg.connect("dbname=dev user=root host=127.0.0.1 port=
with conn.cursor() as cur:
      cur.execute("select (2, '%s');", ("2333333",))
      results = cur.fetchall()
print(results)
conn.close()

Expected behavior

Python script output:

(2, 2333333)

RisingWave work as expected.

Additional context

Actually when I first discovered this problem, it came from a periodic SQL from an unknown source (suspected some periodic check of pg client?) I located this SQL because my RisingWave running on M1 crashes periodically because of it.

Note that this is not the original SQL looks like, the “$” token implies this SQL has been through some parser processing, but not completed. Before the parser processing completes, the RisingWave frontend will crash.

select 
	TABLE_NAME as VIEW_NAME, 
	null as VIEW_TYPE_OWNER, 
	null as VIEW_TYPE, 
case when TABLE_TYPE = 'VIEW' then 'N' else 'Y' end as IS_SYSTEM_VIEW 
from INFORMATION_SCHEMA.TABLES 
	where TABLE_SCHEMA = $1 and TABLE_TYPE in ('VIEW', 'SYSTEM VIEW') 
	order by TABLE_NAME asc;

I cannot find further information about such a query. When I reproduce this issue, since RisingWave only supports varchar field descriptor now, and does not recognize the type of descriptors effectively, I did some changes to bypass the type check.

Navigate to `pg_field_descriptors.rs’

impl TypeOid {
    // TODO: support more type.
    pub fn as_type(oid: i32) -> Result<TypeOid, String> {
        tracing::error!(oid);
        match oid {
            # 
            0 => Ok(TypeOid::Varchar),
            1043 => Ok(TypeOid::Varchar),
            _ => {
                tracing::error!("unsupported typeoid: {}", oid);
                todo!()
            },
        }
    }

Note that this code change is only to help locate the exact problem. When I first discovered this problem, it came from a periodic SQL from an unknown source (suspected some periodic check of pg client?)

select 
	TABLE_NAME as VIEW_NAME, 
	null as VIEW_TYPE_OWNER, 
	null as VIEW_TYPE, 
case when TABLE_TYPE = 'VIEW' then 'N' else 'Y' end as IS_SYSTEM_VIEW 
from INFORMATION_SCHEMA.TABLES 
	where TABLE_SCHEMA = $1 and TABLE_TYPE in ('VIEW', 'SYSTEM VIEW') 
	order by TABLE_NAME asc;

The SQL above will not execute in a command line (SQL parser shows error because a query cannot have $ token as plain literal). The “$” token seems to be a query transform made by the PG client when processing field descriptors.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 2
  • Comments: 16 (16 by maintainers)

Most upvoted comments

I think this issue can be closed by #4900 ? @BowenXiao1999 @KivenChen

So the problem should be parsing ${0…9} as index, nothing relates to select (2, ‘%s’);.

Yes. select (2,‘%s’) will be convert to select (2,‘$1’) and we can’t parse this statement.

I’ll take a time to investigate it🤔

The reason is for now our “extended query mode with params” is too simple. Here is our parser to process statement with params.It’s only support the format like (“select $1,$2,$3”)…

sql.split(&[' ', ',', ';'])
                    .skip(1)
                    .into_iter()
                    .take_while(|x| !x.is_empty())
                    .map(|x| {
                        // NOTE: Assume all output are generic params.
                        let str = x.strip_prefix('$').unwrap();
                        // NOTE: Assume all generic are valid.
                        let v: i32 = str.parse().unwrap();
                        assert!(v.is_positive());
                        v
                    })
                    .map(|x| {
                        // NOTE Make sure the type_description include all generic parameter
                        // description we needed.
                        assert!(((x - 1) as usize) < types.len());
                        PgFieldDescriptor::new(String::new(), types[(x - 1) as usize].to_owned())
                    })
                    .collect()

It’s time to rethink a parser to process more complex statement with params.