cockroachdb is compatible with the postgres wire protocol but not with all the postgres internals. Many other postgres drivers work fine with cockroachdb, but this one doesnt!

Error:

thread '' panicked at 'called Result::unwrap() on an Err value: Io(Error { repr: Custom(Custom { kind: InvalidInput, error: StringError("the server returned an unexpected response") }) })', ../src/libcore/result.rs:688

Repro:

wget https://binaries.cockroachdb.com/cockroach-beta-20160407.linux-amd64.tgz && tar xfz cockroach-beta-20160407.linux-amd64.tgz && cd cockroach-beta-20160407.linux-amd64
./cockroach start
./cockroach sql 
> CREATE DATABASE root;

 let conn = Connection::connect("postgresql://root@localhost:26257/root?sslmode=disable", SslMode::None) .unwrap();
    println!("connection established");
    conn.execute("CREATE TABLE accounts (id INT PRIMARY KEY, balance INT)", &[]).unwrap();
    println!("executed statement");

Since Postgres does not support multiplexing / concurrent queries on a single connection, do you have ideas about how to handle multiple connections?

Would r2d2 be a good fit? I don't know how r2d2 manages the connection internally. Does it use an extra thread or just does maintainance whenever a new connection is acquired?

Does r2d2 fit into this scenario?

Would it make sense to come up with a generic pool manager that is designed to be run on an event loop?

Other thougthts?

Hello, sorry if this is a simple answer. I have a connection to my local PostgreSQL db, and I'm just making simple queries:

println!("Sending query...");
let stmt = &conn.query("SELECT * FROM users", &[])?;
println!("Got response!...");

However, stmt never seems to return from the query. Is there something else that needs to be done?

On windows openssl is not using system certificate store. Even if you have postgresql server secured with qualified certificate, you need to provide its qualified CA certificate for successful handshake. This CA certificate should be in system certificate store. Solution is to use native tls implementation which have access to system certificate store.

I just started dipping my feet into actix water and wanted to go full async. After having a look at existing connection pools I tried both bb8 and the experimental l337 connection pool just to find that they both perform worse than r2d2 + postgres + web::block:

https://bitbucket.org/bikeshedder/actix_web_async_postgres/src/master/

I did not expect those results and wonder if I'm doing anything wrong? I surely did expect tokio-postgres to outperform postgres with ease. Am I doing anything wrong? Are the pools causing the performance drop or is tokio-postgres the culprit?

I have yet to create a benchmark between tokio-postgres and postgres without the use of connection pools. Just posting this here to document my progress in the hope someone can tell if I'm doing anything wrong.

I added a GenericClient trait over Client and Transaction, and replaced &mut with & to support the same style of trait as in previous versions.

I'm using GenericConnection in the old versions and I just wanted to try to add it back. I just named it GenericClient to match the new Client struct.

Thoughts?

Related issue: #329

This should be as simple as doing the following:

• using tokio_tcp::TcpStream instead of tokio_core::tcp::TcpStream
• using tokio_timer::Interval instead of tokio_core::reactor::Interval
• dropping explicit references to Handle

PgBouncer seems to implement some, but not all, of the postgres protocol. Specifically, I was running into this error when trying to query it:

Db(DbError {
    severity: "ERROR",
    parsed_severity: None,
    code: ProtocolViolation,
    message: "unsupported pkt type: 80",
    detail: None,
    hint: None,
    position: None,
    where_: None,
    schema: None,
    table: None,
    column: None,
    datatype: None,
    constraint: None,
    file: None,
    line: None,
    routine: None
})

The error message is from PgBouncer internals (admin.c:1365). Basically, looks like the 'P' frame isn't handled. This is generated from postgres_protocol::messages::frontend::parse.

Any suggestions on how to resolve?

(I'm still working actively on understanding this problem, but I wanted to file a bug upstream.)

Thank you for a great library and for all your recent work on making it async!

Using the dbcrossbar master branch and tokio-postgres 0.4.0-rc.2 / tokio-postgres-native-tls v0.1-rc.1, we're seeing easy-to-reproduce hangs in copy_in when working with large data sets. I can provide sample data sets and instructions if we need those to debug.

In particular, the CopyIn state machine sets state.pending_message:

            match state.sender.start_send(message) {
                Ok(AsyncSink::Ready) => {}
                Ok(AsyncSink::NotReady(message)) => {
                    state.pending_message = Some(message);
                    return Ok(Async::NotReady);
                }

...but we never re-enter the poll function here again:

    fn poll_write_copy_data<'a>(
        state: &'a mut RentToOwn<'a, WriteCopyData<S>>,
    ) -> Poll<AfterWriteCopyData, Error> {

I've tried adding logging, adjusting channel buffer sizes, forcing notify every time through the loop etc., but so far nothing helps. This might be some kind of read/write deadlock, but I'm grasping at straws.

I hope to follow this up with more details as I figure them out.

Thank you very much for any leads you can provide. I'm happy to add instrumentation, try experiments, etc.

This is my very first Rust code ever, it might be completely wrong, however, I wonder why it is slower than Ruby.

extern crate postgres;

use std::rand;

use postgres::{Connection, SslMode};


fn main() {
    println!("Hello, world!");

    let conn = Connection::connect("postgres://mage@localhost", &SslMode::None).unwrap();

    let cycles = 101i32;
    let users = 1001i32;

    for cycle in range(1, cycles) {
        let query = format!("insert into user_daily_statistics (user_id, created_on, web_login) values ($1, now() + '{} days'::interval, $2)", cycle);

        for user_id in range(1, users) {
            let web_login = ((rand::random::<uint>() % 1000u) + 1u) as i32;

            conn.execute(query.as_slice(), &[&user_id, &web_login]).unwrap();
        }
        println!("Done: {}", &cycle);
    }
}

require 'pg'

conn = PG.connect(dbname: 'mage', host: 'localhost')

1.upto(100) do |cycle|
    query = "insert into user_daily_statistics (user_id, created_on, web_login) values ($1, now() + '#{cycle} days'::interval, $2)"
    1.upto(1000) do |user_id|
        conn.exec_params(query, [user_id, rand(1000)])
    end
    puts "Done: #{cycle}"
end

$ time cargo run
Done: 99
Done: 100

real    0m38.003s
user    0m12.195s
sys 0m5.159s

$ time ruby postgres.rb
Done: 99
Done: 100

real    0m22.232s
user    0m2.351s
sys 0m2.495s

The Ruby version runs for 16s when using socket. The Rust version runs for 30 when using socket.

---- Update ----

Change to std-futures branch, but not works...

---- Origin ----

Hi, I found that async-trait macro will throw dyn tokio_postgres::types::ToSql cannot be shared between threads safely when I use tokio-postgres with it together. A demo behind:

Dependencies:

[dependencies]
futures-preview = { version = "=0.3.0-alpha.18", features = ["async-await", "nightly"] }
async-trait = "~0.1.13"
tokio-postgres = { git = "https://github.com/sfackler/rust-postgres", branch="std-futures", features = ["with-uuid-0_7", "with-chrono-0_4", "with-serde_json-1"] }
tokio = "=0.2.0-alpha.4"

extern crate async_trait;

use async_trait::async_trait;
use tokio_postgres::{Client, NoTls, Row};
use futures::{FutureExt, TryStreamExt};
use std::error::Error;

pub struct Thing<'a>(pub &'a str);

#[async_trait]
pub trait Repository {
    async fn get_by_id() -> Option<()>;
}

pub struct RealRepo();


#[async_trait]
impl <'a> Repository for RealRepo {
    async fn get_by_id() -> Option<()> {
        let (mut client, conn) = tokio_postgres::connect("postgres://postgres@localhost", NoTls).await.ok()?;
        let conn = conn.map(|r| {
            if let Err(e) = r {
                eprintln!("connection error: {}", e);
            }
        });

        tokio::spawn(conn);

        let stmt = client.prepare("SELECT * FROM blogs").await.ok()?;
        let res: Vec<&'a str> = client
            .query(&stmt, &[])
            .map_ok(|row: Row| row.get::<usize, &'a str>(0))
            .try_collect()
            .await
            .ok()?;

        Some(())
    }
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let thing = RealRepo::get_by_id().await;
    Ok(())
}

The Error info:

error[E0277]: `dyn tokio_postgres::types::ToSql` cannot be shared between threads safely
  --> src\main.rs:18:1
   |
18 | #[async_trait]
   | ^^^^^^^^^^^^^^ `dyn tokio_postgres::types::ToSql` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `dyn tokio_postgres::types::ToSql`
   = note: required because of the requirements on the impl of `std::marker::Send` for `&dyn tokio_postgres::types::ToSql`
   = note: required because it appears within the type `[&dyn tokio_postgres::types::ToSql; 0]`
   = note: required because it appears within the type `for<'r, 's, 't0, 't1, 't2, 't3, 't4, 't5, 't6, 't7, 't8, 't9, 't10, 't11, 't12, 't13, 't14, 't15, 't16, 't17, 't18, 't19, 't20, 't21, 't22, 't23, 't24, 't25, 't26, 't27, 't28, 't29> {f
n(std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)>) -> std::result::Result<<std::option::Option<(tokio_postgres::client::Client,
tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::Ok, <std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::s
ocket::Socket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::Error> {<std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)> a
s std::ops::Try>::into_result}, for<'_> fn(&str, tokio_postgres::tls::NoTls) -> impl core::future::future::Future {tokio_postgres::connect::<tokio_postgres::tls::NoTls>}, &'r str, str, &'s str, tokio_postgres::tls::NoTls, impl core::future:
:future::Future, impl core::future::future::Future, (), tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>, futures_util::future::map::Map<tokio_postgres:
:connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>, [closure@src\main.rs:22:29: 26:10]>, fn(std::option::Option<tokio_postgres::statement::Statement>) -> std::result::Result<<std::option::Option<tokio_
postgres::statement::Statement> as std::ops::Try>::Ok, <std::option::Option<tokio_postgres::statement::Statement> as std::ops::Try>::Error> {<std::option::Option<tokio_postgres::statement::Statement> as std::ops::Try>::into_result}, &'t3 mu
t tokio_postgres::client::Client, &'t4 str, impl core::future::future::Future, tokio_postgres::statement::Statement, fn(std::option::Option<std::vec::Vec<&'t5 str>>) -> std::result::Result<<std::option::Option<std::vec::Vec<&'t5 str>> as st
d::ops::Try>::Ok, <std::option::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::Error> {<std::option::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::into_result}, &'t6 mut tokio_postgres::client::Client, &'t7 tokio_postgres::statemen
t::Statement, &'t8 tokio_postgres::statement::Statement, [&'t9 (dyn tokio_postgres::types::ToSql + 't10); 0], &'t11 [&'t12 (dyn tokio_postgres::types::ToSql + 't13); 0], &'t14 [&'t15 (dyn tokio_postgres::types::ToSql + 't16); 0], &'t17 [&'t
18 (dyn tokio_postgres::types::ToSql + 't19)], impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60], futures_util::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, futures_ut
il::try_stream::try_collect::TryCollect<futures_util::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, std::vec::Vec<&'t26 str>>, futures_util::try_stream::try_collect::TryCollect<futures_uti
l::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, std::vec::Vec<&'t29 str>>}`
   = note: required because it appears within the type `[static generator@src\main.rs:20:40: 39:6 for<'r, 's, 't0, 't1, 't2, 't3, 't4, 't5, 't6, 't7, 't8, 't9, 't10, 't11, 't12, 't13, 't14, 't15, 't16, 't17, 't18, 't19, 't20, 't21, 't22, 't
23, 't24, 't25, 't26, 't27, 't28, 't29> {fn(std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)>) -> std::result::Result<<std::option
::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::Ok, <std::option::Option<(tokio_postgres::client::Client, tokio_postgres
::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::Error> {<std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Soc
ket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::into_result}, for<'_> fn(&str, tokio_postgres::tls::NoTls) -> impl core::future::future::Future {tokio_postgres::connect::<tokio_postgres::tls::NoTls>}, &'r str, str, &'s str, toki
o_postgres::tls::NoTls, impl core::future::future::Future, impl core::future::future::Future, (), tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>, futu
res_util::future::map::Map<tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>, [closure@src\main.rs:22:29: 26:10]>, fn(std::option::Option<tokio_postgres::statement::Statement>) -> std::
result::Result<<std::option::Option<tokio_postgres::statement::Statement> as std::ops::Try>::Ok, <std::option::Option<tokio_postgres::statement::Statement> as std::ops::Try>::Error> {<std::option::Option<tokio_postgres::statement::Statement
> as std::ops::Try>::into_result}, &'t3 mut tokio_postgres::client::Client, &'t4 str, impl core::future::future::Future, tokio_postgres::statement::Statement, fn(std::option::Option<std::vec::Vec<&'t5 str>>) -> std::result::Result<<std::opt
ion::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::Ok, <std::option::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::Error> {<std::option::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::into_result}, &'t6 mut tokio_postgres::cli
ent::Client, &'t7 tokio_postgres::statement::Statement, &'t8 tokio_postgres::statement::Statement, [&'t9 (dyn tokio_postgres::types::ToSql + 't10); 0], &'t11 [&'t12 (dyn tokio_postgres::types::ToSql + 't13); 0], &'t14 [&'t15 (dyn tokio_post
gres::types::ToSql + 't16); 0], &'t17 [&'t18 (dyn tokio_postgres::types::ToSql + 't19)], impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60], futures_util::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [clos
ure@src\main.rs:33:21: 33:60]>, futures_util::try_stream::try_collect::TryCollect<futures_util::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, std::vec::Vec<&'t26 str>>, futures_util::try_s
tream::try_collect::TryCollect<futures_util::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, std::vec::Vec<&'t29 str>>}]`
   = note: required because it appears within the type `std::future::GenFuture<[static generator@src\main.rs:20:40: 39:6 for<'r, 's, 't0, 't1, 't2, 't3, 't4, 't5, 't6, 't7, 't8, 't9, 't10, 't11, 't12, 't13, 't14, 't15, 't16, 't17, 't18, 't1
9, 't20, 't21, 't22, 't23, 't24, 't25, 't26, 't27, 't28, 't29> {fn(std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)>) -> std::resu
lt::Result<<std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::Ok, <std::option::Option<(tokio_postgres::client:
:Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::Error> {<std::option::Option<(tokio_postgres::client::Client, tokio_postgres::connection::Connection<toki
o_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>)> as std::ops::Try>::into_result}, for<'_> fn(&str, tokio_postgres::tls::NoTls) -> impl core::future::future::Future {tokio_postgres::connect::<tokio_postgres::tls::NoTls>}, &'r
str, str, &'s str, tokio_postgres::tls::NoTls, impl core::future::future::Future, impl core::future::future::Future, (), tokio_postgres::client::Client, tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::
tls::NoTlsStream>, futures_util::future::map::Map<tokio_postgres::connection::Connection<tokio_postgres::socket::Socket, tokio_postgres::tls::NoTlsStream>, [closure@src\main.rs:22:29: 26:10]>, fn(std::option::Option<tokio_postgres::statemen
t::Statement>) -> std::result::Result<<std::option::Option<tokio_postgres::statement::Statement> as std::ops::Try>::Ok, <std::option::Option<tokio_postgres::statement::Statement> as std::ops::Try>::Error> {<std::option::Option<tokio_postgre
s::statement::Statement> as std::ops::Try>::into_result}, &'t3 mut tokio_postgres::client::Client, &'t4 str, impl core::future::future::Future, tokio_postgres::statement::Statement, fn(std::option::Option<std::vec::Vec<&'t5 str>>) -> std::r
esult::Result<<std::option::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::Ok, <std::option::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::Error> {<std::option::Option<std::vec::Vec<&'t5 str>> as std::ops::Try>::into_result}, &'t6
mut tokio_postgres::client::Client, &'t7 tokio_postgres::statement::Statement, &'t8 tokio_postgres::statement::Statement, [&'t9 (dyn tokio_postgres::types::ToSql + 't10); 0], &'t11 [&'t12 (dyn tokio_postgres::types::ToSql + 't13); 0], &'t14
 [&'t15 (dyn tokio_postgres::types::ToSql + 't16); 0], &'t17 [&'t18 (dyn tokio_postgres::types::ToSql + 't19)], impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60], futures_util::try_stream::map_ok::MapOk<impl futures_core
::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, futures_util::try_stream::try_collect::TryCollect<futures_util::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, std::vec::Vec<&'t26 str
>>, futures_util::try_stream::try_collect::TryCollect<futures_util::try_stream::map_ok::MapOk<impl futures_core::stream::Stream, [closure@src\main.rs:33:21: 33:60]>, std::vec::Vec<&'t29 str>>}]>`
   = note: required because it appears within the type `impl core::future::future::Future`
   = note: required because it appears within the type `impl core::future::future::Future`
   = note: required for the cast to the object type `dyn core::future::future::Future<Output = std::option::Option<()>> + std::marker::Send`

Updates the requirements on base64 to permit the latest version.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Added RowDescription trait, and let rows to share the same description rather than having a copy in each row (think when there are thousand of them in the result).

Added RowBuilder to support adding stubs of row data in unit tests.

Currently, the library users have no chooice but have to use integration tests for testing Postgres data access code. With the changes in this commit, the tokio-postgres lib users can use RowBuilder to create sutbs to verify the deserialization from database result (Rows) to custom stucts in unit tests.

It can also serves as a base for future implementation of certain kind of mocks of the db connection.

Related-to #910 #950

Hi. I'd like to fetch rows directly into a struct, without going field by field and doing let field1: MyType = row.get(0) etc. Something like let person: Person = row.get_struct(), which can reuse the composite type logic to parse into a struct deriving FromSql.

You can currently hack around this with the following trick. Here, I'm using the fact that Postgres already defines a composite type for every table; for this trick to work with other than SELECT *, you'd need to define a PG composite type for each query too.

#[derive(Debug, ToSql, FromSql)]
#[postgres(name = "person")]
struct Person {
    id: i64,
    name: String,
    data: Option<Vec<u8>>,
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut client = postgres::Client::connect(
        "host=localhost user=postgres",
        postgres::NoTls,
    )?;

    client.batch_execute(
        "
        CREATE TABLE IF NOT EXISTS person (
            id      BIGSERIAL PRIMARY KEY,
            name    TEXT NOT NULL,
            data    BYTEA
        )
    ",
    )?;

    {
        let name = "Ferris";
        client.execute(
            "INSERT INTO person (name) VALUES ($1)",
            &[&name],
        )?;
    }

    for row in client.query(
        // Jump through hoops to make postgres-types work well.
        "SELECT row(person.*)::person FROM person",
        &[],
    )? {
        let person: Person = row.get(0);
        println!("found person: {:?}", person);
    }

It'd be nice to avoid that extra row(...)::person trickery, and to not need to define Postgres-side composite types for every possible combination of selected columns. postgres-types should be able to enumerate the columns and match each one to a struct field, just like it does with composite types (without needing a composite type).

I think it's too late to change now, but just wanted to mention.

Since tokio_postgres::Error duplicates its cause in Display:

if let Some(ref cause) = self.0.cause {
    write!(fmt, ": {}", cause)?;
}

... this means that things like anyhow::Error (with :#) that walk the cause chain and concatenate the description will duplicate the message, e.g.:

db error: ERROR: syntax error at or near "SELECTa": ERROR: syntax error at or near "SELECTa"

Ideally, it would either

• include cause and only print things relevant to itself in the description, or
• not include cause, and include it in its own description, or
• support both modes like anyhow::Error (concatenate only in alternate mode).

Context

Here is an example scenario:

// getting the iterator
let reader = client.copy_out(&*query)?; // blocked when query is complex
let iter = BinaryCopyOutIter::new(reader, schema);

// some other logic X, may take some time
...

// traverse the result
loop {
    let res = iter.next()?;
    ...
}

It works great when the query is simple (e.g., select * from a table). However, when it gets complex (like this tpc-h query). Getting the reader will block for several seconds.

The reason for this is that currently copy_out needs to wait for receiving BindComplete and CopyOutResponse message to return. These two messages are indeed extremely fast to generate in postgres, but since there is no manual flush logic, they are not returned to the client until postgres generate some execution result to the buffer and make it reach to the flush threshold. Therefore, when query becomes complex and need more time to produce the first few result tuples, getting the CopyOutReader will be blocked.

The issue caused by this is that it will extend the end-to-end time for this example. Since the total time used before we retrieve the result would be time(postgres execution) + time(logic X).

Propose solution

A simple solution is to add a Flush message after Bind in order to get BindComplete immediately. And delay the receiving for CopyOutResponse to result retrieval. This will unblock copy_out and make the above example from time(postgres execution) + time(logic X) to MAX(time(postgres execution), time(logic X)).

Here is a PR for this: #971. Please let me know if I missed anything, thanks!