adbc_postgresql: core dump if server terminates the connection

[CaselIT]

What happened?

If the postgresql server terminates the connection the processes crashes with a core dump

Stack Trace

terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_S_construct null not valid
Aborted (core dumped)

How can we reproduce the bug?

from adbc_driver_postgresql.dbapi import connect

uri = "postgresql://scott:tiger@localhost:5432/test"

conn = connect(uri)
conn2 = connect(uri)
c2Pid = conn2.execute("select pg_backend_pid()").fetchone()[0]
conn2.rollback()
print(c2Pid)
# just an example to simulate a server side termination of a connection
conn.execute("select pg_terminate_backend($1::int)", (c2Pid,))
print("terminated")
conn2.execute("select 1")
print("crashes above")

Environment/Setup

adbc-driver-manager 1.10.0 or 1.9.0
adbc-driver-postgresql 1.10.0 or 1.9.0
Os: windows or linux
Postgresql: 18 (but it should not matter)

[DavideCanton]

Probably the error is here https://github.com/apache/arrow-adbc/blob/main/c/driver/postgresql/error.h#L86, if result is null then sqlstate is null too sqlstate can be null, but below is passed to status.SetSqlState that implicitly tries to convert it to a std::string

[CaselIT]

It seems there was a typo in the refactoring done here a48ccec

It went from

  const char* sqlstate = PQresultErrorField(result, PG_DIAG_SQLSTATE);
  if (sqlstate) {
    // https://www.postgresql.org/docs/current/errcodes-appendix.html
    // This can be extended in the future
    if (std::strcmp(sqlstate, "57014") == 0) {
      code = ADBC_STATUS_CANCELLED;
    } else if (std::strcmp(sqlstate, "42P01") == 0 ||
               std::strcmp(sqlstate, "42602") == 0) {
      code = ADBC_STATUS_NOT_FOUND;
    } else if (std::strncmp(sqlstate, "42", 0) == 0) {
      // Class 42 — Syntax Error or Access Rule Violation
      code = ADBC_STATUS_INVALID_ARGUMENT;
    }

    static_assert(sizeof(error->sqlstate) == 5, "");
    // N.B. strncpy generates warnings when used for this purpose
    int i = 0;
    for (; sqlstate[i] != '\0' && i < 5; i++) {
      error->sqlstate[i] = sqlstate[i];
    }
    for (; i < 5; i++) {
      error->sqlstate[i] = '\0';
    }
  }

to

  if (sqlstate) {
    // https://www.postgresql.org/docs/current/errcodes-appendix.html
    // This can be extended in the future
    if (std::strcmp(sqlstate, "57014") == 0) {
      code = ADBC_STATUS_CANCELLED;
    } else if (std::strcmp(sqlstate, "42P01") == 0 ||
               std::strcmp(sqlstate, "42602") == 0) {
      code = ADBC_STATUS_NOT_FOUND;
    } else if (std::strncmp(sqlstate, "42", 0) == 0) {
      // Class 42 — Syntax Error or Access Rule Violation
      code = ADBC_STATUS_INVALID_ARGUMENT;
    }
  }

  Status status(code, message);
  status.SetSqlState(sqlstate);

SetSqlStates hould be called only inside the if (sqlstate) { not outside

[lidavidm]

CC @Mandukhai-Alimaa

[Mandukhai-Alimaa]

take