Skip to content

Commit

Permalink
feat(go/adbc/driver/flightsql): reflect gRPC status in vendor code (a…
Browse files Browse the repository at this point in the history
…pache#1577)

Does not work in C/C++/Python due to apache#1576.

Fixes apache#1574.
  • Loading branch information
lidavidm authored Mar 1, 2024
1 parent 56e96e2 commit bcbc161
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 7 deletions.
63 changes: 63 additions & 0 deletions go/adbc/driver/flightsql/flightsql_adbc_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,16 @@ func (srv *ErrorDetailsTestServer) GetFlightInfoStatement(ctx context.Context, q
panic(err)
}
return &flight.FlightInfo{Endpoint: []*flight.FlightEndpoint{{Ticket: &flight.Ticket{Ticket: tkt}}}}, nil
} else if query.GetQuery() == "vendorcode" {
return nil, status.Errorf(codes.ResourceExhausted, "Resource exhausted")
} else if query.GetQuery() == "binaryheader" {
if err := grpc.SendHeader(ctx, metadata.Pairs("x-header-bin", string([]byte{0, 110}))); err != nil {
return nil, err
}
if err := grpc.SetTrailer(ctx, metadata.Pairs("x-trailer-bin", string([]byte{111, 0, 112}))); err != nil {
return nil, err
}
return nil, status.Errorf(codes.FailedPrecondition, "Resource exhausted")
}
return nil, status.Errorf(codes.Unimplemented, "GetSchemaStatement not implemented")
}
Expand Down Expand Up @@ -287,6 +297,43 @@ func (suite *ErrorDetailsTests) SetupSuite() {
suite.DoSetupSuite(&srv, nil, nil)
}

func (ts *ErrorDetailsTests) TestBinaryDetails() {
stmt, err := ts.cnxn.NewStatement()
ts.NoError(err)
defer stmt.Close()

ts.NoError(stmt.SetSqlQuery("binaryheader"))

_, _, err = stmt.ExecuteQuery(context.Background())
var adbcErr adbc.Error
ts.ErrorAs(err, &adbcErr)

ts.Equal(int32(codes.FailedPrecondition), adbcErr.VendorCode)

ts.Equal(2, len(adbcErr.Details))

headerFound := false
trailerFound := false
for _, wrapper := range adbcErr.Details {
switch wrapper.Key() {
case "x-header-bin":
val, err := wrapper.Serialize()
ts.NoError(err)
ts.Equal([]byte{0, 110}, val)
headerFound = true
case "x-trailer-bin":
val, err := wrapper.Serialize()
ts.NoError(err)
ts.Equal([]byte{111, 0, 112}, val)
trailerFound = true
default:
ts.Failf("Unexpected detail key: %s", wrapper.Key())
}
}
ts.Truef(headerFound, "Did not find x-header-bin")
ts.Truef(trailerFound, "Did not find x-trailer-bin")
}

func (ts *ErrorDetailsTests) TestGetFlightInfo() {
stmt, err := ts.cnxn.NewStatement()
ts.NoError(err)
Expand All @@ -298,6 +345,8 @@ func (ts *ErrorDetailsTests) TestGetFlightInfo() {
var adbcErr adbc.Error
ts.ErrorAs(err, &adbcErr)

ts.Equal(int32(codes.Unknown), adbcErr.VendorCode)

ts.Equal(1, len(adbcErr.Details))

wrapper := adbcErr.Details[0]
Expand Down Expand Up @@ -347,6 +396,20 @@ func (ts *ErrorDetailsTests) TestDoGet() {
ts.Equal(int32(42), message.Value)
}

func (ts *ErrorDetailsTests) TestVendorCode() {
stmt, err := ts.cnxn.NewStatement()
ts.NoError(err)
defer stmt.Close()

ts.NoError(stmt.SetSqlQuery("vendorcode"))

_, _, err = stmt.ExecuteQuery(context.Background())
var adbcErr adbc.Error
ts.ErrorAs(err, &adbcErr)

ts.Equal(int32(codes.ResourceExhausted), adbcErr.VendorCode)
}

// ---- ExecuteSchema Tests --------------------

type ExecuteSchemaTestServer struct {
Expand Down
34 changes: 27 additions & 7 deletions go/adbc/driver/flightsql/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package flightsql
import (
"context"
"fmt"
"strings"

"github.com/apache/arrow-adbc/go/adbc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -92,19 +93,37 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con
// XXX: must check both headers and trailers because some implementations
// (like gRPC-Java) will consolidate trailers into headers for failed RPCs
for key, values := range header {
switch key {
case "content-type", "grpc-status-details-bin":
switch {
case key == "content-type":
// Not useful info
continue
case key == "grpc-status-details-bin":
// gRPC library parses this above via grpcStatus.Proto()
continue
case strings.HasSuffix(key, "-bin"):
for _, value := range values {
// that's right, gRPC stuffs binary data into a "string"
details = append(details, &adbc.BinaryErrorDetail{Name: key, Detail: []byte(value)})
}
default:
for _, value := range values {
details = append(details, &adbc.TextErrorDetail{Name: key, Detail: value})
}
}
}
for key, values := range trailer {
switch key {
case "content-type", "grpc-status-details-bin":
switch {
case key == "content-type":
// Not useful info
continue
case key == "grpc-status-details-bin":
// gRPC library parses this above via grpcStatus.Proto()
continue
case strings.HasSuffix(key, "-bin"):
for _, value := range values {
// that's right, gRPC stuffs binary data into a "string"
details = append(details, &adbc.BinaryErrorDetail{Name: key, Detail: []byte(value)})
}
default:
for _, value := range values {
details = append(details, &adbc.TextErrorDetail{Name: key, Detail: value})
Expand All @@ -114,9 +133,10 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con

return adbc.Error{
// People don't read error messages, so backload the context and frontload the server error
Msg: fmt.Sprintf("[FlightSQL] %s (%s; %s)", grpcStatus.Message(), grpcStatus.Code(), fmt.Sprintf(context, args...)),
Code: adbcCode,
Details: details,
Msg: fmt.Sprintf("[FlightSQL] %s (%s; %s)", grpcStatus.Message(), grpcStatus.Code(), fmt.Sprintf(context, args...)),
Code: adbcCode,
VendorCode: int32(grpcStatus.Code()),
Details: details,
}
}

Expand Down
3 changes: 3 additions & 0 deletions python/adbc_driver_flightsql/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,6 @@ include-package-data = true
license-files = ["LICENSE.txt", "NOTICE.txt"]
packages = ["adbc_driver_flightsql"]
py-modules = ["adbc_driver_flightsql"]

[tool.pytest.ini_options]
xfail_strict = true
15 changes: 15 additions & 0 deletions python/adbc_driver_flightsql/tests/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,21 @@ def test_query_error_fetch(test_dbapi):
assert_detail(excval.value)


@pytest.mark.xfail(reason="apache/arrow-adbc#1576")
def test_query_error_vendor_code(test_dbapi):
with test_dbapi.cursor() as cur:
cur.execute("error_do_get")
with pytest.raises(
test_dbapi.ProgrammingError,
match=re.escape("INVALID_ARGUMENT: [FlightSQL] expected error (DoGet)"),
) as excval:
cur.fetch_arrow_table()

# TODO(https://github.com/apache/arrow-adbc/issues/1576): vendor code
# is gRPC status code; 3 is gRPC INVALID_ARGUMENT
assert excval.value.vendor_code == 3


def test_query_error_stream(test_dbapi):
with test_dbapi.cursor() as cur:
cur.execute("error_do_get_stream")
Expand Down
1 change: 1 addition & 0 deletions python/adbc_driver_manager/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ markers = [
"panicdummy: tests that require the testing-only panicdummy driver",
"sqlite: tests that require the SQLite driver",
]
xfail_strict = true

[tool.setuptools]
license-files = ["LICENSE.txt", "NOTICE.txt"]
Expand Down
1 change: 1 addition & 0 deletions python/adbc_driver_postgresql/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ build-backend = "setuptools.build_meta"
markers = [
"polars: integration tests with polars",
]
xfail_strict = true

[tool.setuptools]
include-package-data = true
Expand Down
3 changes: 3 additions & 0 deletions python/adbc_driver_snowflake/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,6 @@ include-package-data = true
license-files = ["LICENSE.txt", "NOTICE.txt"]
packages = ["adbc_driver_snowflake"]
py-modules = ["adbc_driver_snowflake"]

[tool.pytest.ini_options]
xfail_strict = true
3 changes: 3 additions & 0 deletions python/adbc_driver_sqlite/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,6 @@ include-package-data = true
license-files = ["LICENSE.txt", "NOTICE.txt"]
packages = ["adbc_driver_sqlite"]
py-modules = ["adbc_driver_sqlite"]

[tool.pytest.ini_options]
xfail_strict = true

0 comments on commit bcbc161

Please sign in to comment.