python: segfault when passing RecordBatchReader to PyArrow

[lupko]

Hello,

I'm trying to integrate ADBC into a Flight RPC service. Typical use case is that a call (GetFlightInfo) performs a query and then client comes to pick stream of results via DoGet. So on GetFlightInfo service creates new cursor, executes and sends appropriate FlightInfo so that DoGet with the right ticket will pick up the data. For this the code does cursor.fetch_record_batch(). The resulting RecordBatchReader is then wrapped into pyarrow.flight.RecordBatchStream and returned.

Doing this crashes the entire server with SIGSEGV. You can find the reproducer in this gist: https://gist.github.com/lupko/8b6f165a6574ef830c531c8056b20957. The reproducer skips the GetFlightInfo for sakes of brevity.

Poking around the code of ADBC python wrappers, I think this crash happens because AdbcRecordBatchReader is not ready for interop with PyArrow .

The PyArrow's RecordBatchReader (pyarrow.lib.RecordBatchReader) has reader field that contains the actual C++ RecordBatchReader. PyArrow code usually grabs the actual reader as soon as possible and uses it for the different purposes (like getting batches to send out via Flight RPC).

The AdbcRecordBatchReader does not extend pyarrow.lib.RecordBatchReader and so reader is just not there and everything comes crashing down.

[lidavidm]

Hmm, the problem is that the PyArrow bindings are intentionally restrictive about how you can extend them, while we want to be able to pass on ADBC errors through this interface, so we have a wrapper...I suppose the wrapper needs to try harder to pretend to be a real reader.

Right now, the DBAPI portion always wraps the stream in a RecordBatchReader. I think what I might try doing instead is have it use the low-level methods directly bypassing PyArrow, and if you instead request the reader, we just give you a plain PyArrow RecordBatchReader (which will not give you rich error metadata because we can't extend this interface in Python). Or I will see if we can finagle it so that we can link to PyArrow in Cython (however, this will mean we're dependent on particular PyArrow versions which I would very much like to avoid).

[lidavidm]

Hmm, and we can't monkeypatch the reader object either, otherwise that would be a viable method :/

[lupko]

As a workaround, I have just noticed that AdbcRecordBatchReader._import_from_c actually uses PyArrow's RBR and keeps it in _reader. If i go dirty and use that fetch_record_batch()._reader then all works as expected. Indeed, not the ideal way to go but it unblocks my progress a bit.

[lidavidm]

Cool. I can reproduce and will get a fix out for the next release. Thanks for the report!

[lidavidm]

The 'fix' will just be to return _reader for you; I've discussed some potential fancier fixes with PyArrow maintainers but this will work for the time being.

[lupko]

thanks @lidavidm