These are two reasons to need the protobuf in python. However, I still don't
fully understand why this is needed for pyarrow.
For #1, the invocations of google.protobuf.json_format.ParseDict() are going to be in
PyArrow unit tests of UDF functionality. As described earlier, I was able to avoid this
invocation until I got a "TypeError: Object of type bytes is not JSON
serializable" for a bytes-type field holding the UDF pickle.
For #2, this is related to the more general preference for PyArrow when dealing
with UDFs. Basically, it is easier for the Substrait-producer to pickle a
Python-implemented UDF into a Substrait plan via PyArrow APIs and, similarly,
for the Substrait-consumer to unpickle the UDF from the Substrait plan via
PyArrow APIs. Doing it via Arrow C++ APIs, using callbacks or by launching an
embedded Python interpreter, is undesirable IMO. More specifically, the use
case of #2 is for UDTs (user-defined tables), which are objects somewhat more
complex than UDFs and are yet easier to deal with using Python code.
In the first approach it would make sense that you would need Substrait access
from python.
The first approach is basically the design I'm advocating, except that the
registrations allow the replacing-step to be avoided, so the Substrait plan
need not be modified. Indeed, this approach leads to a need for Substrait
access from Python (and PyArrow, as explained above).
In the second approach it wouldn't be needed.
Agreed. Though this approach has its disadvantages. For example, each new kind
of user-defined object (like UDT) would require at least one new callback in
Arrow C++ code. It would also be harder for the user to manually register UDFs
outside the context of a Substrait plan, and I think also the Arrow Substrait
APIs would need to be changed to accept a FunctionRegistry in order to support
a nested registry; it's easier, and probably more flexible, to let PyArrow
drive instead of getting called back.
Is this something I might be able to infer from [1] (I haven't yet had a chance
to look at that in detail)?
[1] is focused on UDFs, so it could give you an idea but won't show you the
details of UDT integration.
Yaron.
________________________________
From: Weston Pace <weston.p...@gmail.com>
Sent: Wednesday, July 6, 2022 1:30 PM
To: dev@arrow.apache.org <dev@arrow.apache.org>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
1. Parsing a dictionary using google.protobuf.json_format.ParseDict(). This
method requires an instance of a Substrait protobuf Python class, such as Plan,
to merge with; hence, the need to access such classes, but no need to modify
the Substrait plan. This case was described in the first message in this thread.
2. Accessing a Python object, which is more convenient to manipulate from
Python, after unpickling in from a field in the Substrait plan. It's just
read-only access to the field from Python, but still needs access to the
Substrait protobuf Python classes. This case was mentioned in my previous
message in this thread in response to Li's specific question about UDFs.
These are two reasons to need the protobuf in python. However, I
still don't fully understand why this is needed for pyarrow.
I can maybe try and infer a little. For example, is this required to
add support in pyarrow for running UDFs that are embedded in the
Substrait plan?
One possible implementation could be to:
* Have pyarrow accept the Substrait plan
* Extract the embedded UDFs
* Register Arrow UDFs
* Replace the calls to the UDF in the Substrait plan with calls to
the newly registered UDF
* Submit the modified Substrait plan to Arrow-C++
However, another potential implementation could be to:
* Define a "UDF provider" in C++. This is a callback that receives a
buffer (the pickled UDF) and a name and also a function that creates
an Arrow "call" given the name of a UDF
* Implement the UDF provider in pyarrow
* The implementation would unpickle the UDF and register a UDF
with Arrow. Then making the Arrow call would just be creating a call
into the registered UDF.
* Continue to have Substrait plans go directly into Arrow-C++, but
now pyarrow is a "UDF provider" for "pickle UDFs"
In the first approach it would make sense that you would need
Substrait access from python. In the second approach it wouldn't be
needed. I'm not advocating for one approach vs the other but I am
trying to understand the overall goal with adding this support into
pyarrow. However, a PR may be clearer. Is this something I might be
able to infer from [1] (I haven't yet had a chance to look at that in
detail)?
Regarding Rope that I mentioned earlier in this thread, it has an LGPL v3+
license. Is this license acceptable for a build dependency?
TMK, that should be fine for a build dependency.
[1] https://github.com/apache/arrow/pull/13500
On Wed, Jul 6, 2022 at 5:07 AM Yaron Gvili <rt...@hotmail.com> wrote:
Regarding Rope that I mentioned earlier in this thread, it has an LGPL v3+
license. Is this license acceptable for a build dependency?
Yaron.
________________________________
From: Yaron Gvili <rt...@hotmail.com>
Sent: Wednesday, July 6, 2022 7:26 AM
To: dev@arrow.apache.org <dev@arrow.apache.org>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
@Weston: The need for accessing the Substrait protobuf Python classes is not
for the purpose of modifying the Substrait plan. There are two relevant cases I
went into:
1. Parsing a dictionary using google.protobuf.json_format.ParseDict(). This
method requires an instance of a Substrait protobuf Python class, such as Plan,
to merge with; hence, the need to access such classes, but no need to modify
the Substrait plan. This case was described in the first message in this thread.
2. Accessing a Python object, which is more convenient to manipulate from
Python, after unpickling in from a field in the Substrait plan. It's just
read-only access to the field from Python, but still needs access to the
Substrait protobuf Python classes. This case was mentioned in my previous
message in this thread in response to Li's specific question about UDFs.
Yaron.
________________________________
From: Weston Pace <weston.p...@gmail.com>
Sent: Tuesday, July 5, 2022 7:59 PM
To: dev@arrow.apache.org <dev@arrow.apache.org>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
If this is to be used within pyarrow then I think it makes sense as
something to do. Can you expand a bit more on what you are trying to
do? Why do you need to modify the Substrait plan in pyarrow?
This came up in a data-source UDF scenario where the implementation is a Python
stream factory, i.e., a Python callable returning a callable. I think it would
be easier to get the factory and then the stream using Python code than using
Cython or C++.
I'm not quite certain why this requires a modification to the plan.
On Tue, Jul 5, 2022 at 7:45 AM Yaron Gvili <rt...@hotmail.com> wrote:
@Li, yes though in a new way. This came up in a data-source UDF scenario where
the implementation is a Python stream factory, i.e., a Python callable
returning a callable. I think it would be easier to get the factory and then
the stream using Python code than using Cython or C++.
Yaron.
________________________________
From: Li Jin <ice.xell...@gmail.com>
Sent: Tuesday, July 5, 2022, 4:22 PM
To: dev@arrow.apache.org <dev@arrow.apache.org>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
Yaron, do we need to parse the subtrait protobuf in Python so that we can
get the UDFs and register them with Pyarrow?
On Mon, Jul 4, 2022 at 1:24 PM Yaron Gvili <rt...@hotmail.com> wrote:
This rewriting of the package is basically what I had in mind; the `_ep`
was just to signal a private package, which cannot be enforced, of course.
Assuming this rewriting would indeed avoid conflict with any standard
protobuf package a user might want to use, it would be nicer to do this
using a robust refactoring
tool. We could try Rope [1] (in particular, its folder refactoring library
method [2]) for this; it should add Rope only as a build dependency. Does
this sound worth trying?
[1] https://github.com/python-rope/rope
[2]
https://rope.readthedocs.io/en/latest/library.html#list-of-refactorings
Yaron.
________________________________
From: Jeroen van Straten <jeroen.van.stra...@gmail.com>
Sent: Monday, July 4, 2022 12:38 PM
To: dev@arrow.apache.org <dev@arrow.apache.org>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
Ah, I see. I guess that complicates things a little.
About the exposure part in C++, the idea is that if we statically link
libprotobuf and use private linkage for the generated classes, *hopefully*
users won't run into linking issues when they link to both Arrow and to
some libprotobuf of their own, especially when they or another one of their
dependencies also includes Substrait protobuf. The classes would absolutely
conflict otherwise, and the libprotobuf version may or may not conflict.
The latter is kind of an unsolved problem; AFAIK there have been talks to
replace libprotobuf with a third-party alternative to avoid all this
nonsense, but nothing concrete yet.
For Python, the problems are not quite the same:
- There is no such thing as private linkage in Python, so we *have* to
re-namespace the generated Python files. Note that the output of protoc
assumes that the files are in exactly the same namespace/module
path/whatever as the proto files specify, so the toplevel module would be
"substrait", not "arrow". See
https://github.com/substrait-io/substrait/pull/207 and
https://github.com/substrait-io/substrait-validator/blob/941b81ddf65d8ee6266d6eea0d151a3f9d8a512b/py/build.rs#L105-L142
for my ugly workarounds for this thus far.
- There is also no such thing as static linkage in Python, so if you'd
want to use Google's protobuf implementation in Python pyarrow *will* need
to depend on it, and it'd become the user's problem to make sure that the
installed Python protobuf version matches the version of their system
library. It looks like pyarrow currently only depends on numpy, which is
pretty awesome... so I feel like we should keep it that way.
Not sure what the best course of action is.
Jeroen
On Sun, 3 Jul 2022 at 22:55, Yaron Gvili <rt...@hotmail.com> wrote:
Thanks, the Google protobuf exposure concerns are clear. Another concern
would be consistent maintenance of the same version of Substrait protobuf
used in Arrow C++ and in PyArrow.
I didn't mean access to users but internally. That is, I didn't mean to
expose Substrait protobuf Python classes to PyArrow users, just to use
them
internally in PyArrow code I'll be developing, per the use case I
described
at the start of this thread. IIUC, this should address the exposure
concerns, and the `arrow/engine/substrait` module in Arrow C++ does this
too. If the technical approach I just described would actually expose the
classes, what would be a proper way to avoid exposing them? Perhaps the
classes should be generated into a private package, e.g., under
`python/_ep`? (ep stands for external project)
Yaron.
________________________________
From: Antoine Pitrou <anto...@python.org>
Sent: Sunday, July 3, 2022 3:20 PM
To: dev@arrow.apache.org <dev@arrow.apache.org>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
I agree that giving direct access to protobuf classes is not Arrow's
job. You can probably take the upstream (i.e. Substrait's) protobuf
definitions and compile them yourself, using whatever settings required
by your project.
Regards
Antoine.
Le 03/07/2022 à 21:16, Jeroen van Straten a écrit :
It's not so much about whether or not we *can*, but about whether or
not
we
*should* generate and expose these files.
Fundamentally, Substrait aims to be a means to connect different
systems
together by standardizing some interchange format. Currently that
happens
to be based on protobuf, so *one* (obvious) way to generate and
interpret
Substrait plans is to use Google's own protobuf implementation, as
generated by protoc for various languages. It's true that there's
nothing
fundamentally blocking Arrow from exposing those.
However, it's by no means the *only* way to interact with protobuf
serializations, and Google's own implementation is a hot mess when it
comes
to dependencies; there are lots of good reasons why you might not want
to
depend on it and opt for a third-party implementation instead. For one
thing, the Python wheel depends on system libraries beyond manylinux,
and
if they happen to be incompatible (which is likely) it just explodes
unless
you set some environment variable. Therefore, assuming pyarrow doesn't
already depend on protobuf, I feel like we should keep it that way, and
thus that we should not include the generated Python files in the
release.
Note that we don't even expose/leak the protoc-generated C++ classes
for
similar reasons.
Also, as Weston already pointed out, it's not really our job; Substrait
aims to publish bindings for various languages by itself. It just
doesn't
expose them for Python *yet*. In the interim I suppose you could use
the
substrait-validator package from PyPI. It does expose them, as well as
some
convenient conversion functions, but I'm having trouble finding people
to
help me keep the validator maintained.
I suppose versioning would be difficult either way, since Substrait
semi-regularly pushes breaking changes and Arrow currently lags behind
by
several months (though I have a PR open for Substrait 0.6). I guess
from
that point of view distributing the right version along with pyarrow
seems
nice, but the issues of Google's protobuf implementation remain. This
being
an issue at all is also very much a Substrait problem, not an Arrow
problem; at best we can try to mitigate it.
Jeroen
On Sun, Jul 3, 2022, 17:51 Yaron Gvili <rt...@hotmail.com> wrote:
I looked into the Arrow build system some more. It is possible to get
the
Python classes generated by adding "--python-out" flag (set to a
directory
created for it) to the `${ARROW_PROTOBUF_PROTOC}` command under
`macro(build_substrait)` in
`cpp/cmake_modules/ThirdpartyToolchain.cmake`.
However, this makes them available only in the Arrow C++ build whereas
for
the current purpose they need to be available in the PyArrow build.
The
PyArrow build calls `cmake` on `python/CMakeLists.txt`, which AFAICS
has
access to `cpp/cmake_modules`. So, one solution could be to pull
`macro(build_substrait)` into `python/CMakeLists.txt` and call it to
generate the Python protobuf classes under `python/`, making them
available
for import by PyArrow code. This would probably be cleaner with some
macro
parameters to distinguish between C++ and Python generation.
Does this sound like a reasonable approach?
Yaron.
________________________________
From: Yaron Gvili <rt...@hotmail.com>
Sent: Saturday, July 2, 2022 8:55 AM
To: dev@arrow.apache.org <dev@arrow.apache.org>; Phillip Cloud <
cpcl...@gmail.com>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
I'm somewhat confused by this answer because I think resolving the
issue I
raised does not require any change outside PyArrow. I'll try to
explain
the
issue differently.
First, let me describe the current situation with Substrait protobuf
in
Arrow C++. The Arrow C++ build system handles external projects in
`cpp/cmake_modules/ThirdpartyToolchain.cmake`, and one of these
external
projects is "substrait". By default, the build system takes the source
code
for "substrait" from `
https://github.com/substrait-io/substrait/archive/${ARROW_SUBSTRAIT_BUILD_VERSION}.tar.gz
` where `ARROW_SUBSTRAIT_BUILD_VERSION` is set in
`cpp/thirdparty/versions.txt`. The source code is check-summed and
unpacked
in `substrait_ep-prefix` under the build directory and from this the
protobuf C++ classes are generated in `*.pb.{h,cc}` files in
`substrait_ep-generated` under the build directory. The build system
makes
a library using the `*.cc` files and makes the `*.h` files available
for
other C++ modules to use.
Setting up the above mechanism did not require any change in the
`substrait-io/substrait` repo, nor any coordination with its authors.
What
I'm looking for is a similar build mechanism for PyArrow that builds
Substrait protobuf Python classes and makes them available for use by
other
PyArrow modules. I believe this PyArrow build mechanism does not exist
currently and that setting up one would not require any changes
outside
PyArrow. I'm asking (1) whether that's indeed the case, (2) whether
others
agree this mechanism is needed at least due to the problem I ran into
that
I previously described, and (3) for any thoughts about how to set up
this
mechanism assuming it is needed.
Weston, perhaps your thinking was that the Substrait protobuf Python
classes need to be built by a repo in the substrait-io space and made
available as a binary+headers package? This can be done but will
require
involving Substrait people and appears to be inconsistent with current
patterns in the Arrow build system. Note that for my purposes here,
the
Substrait protobuf Python classes will be used for composing or
interpreting a Substrait plan, not for transforming it by an
optimizer,
though a Python-based optimizer is a valid use case for them.
Yaron.
________________________________
From: Weston Pace <weston.p...@gmail.com>
Sent: Friday, July 1, 2022 12:42 PM
To: dev@arrow.apache.org <dev@arrow.apache.org>; Phillip Cloud <
cpcl...@gmail.com>
Subject: Re: accessing Substrait protobuf Python classes from PyArrow
Given that Acero does not do any planner / optimizer type tasks I'm
not sure you will find anything like this in arrow-cpp or pyarrow.
What you are describing I sometimes refer to as "plan slicing and
dicing". I have wondered if we will someday need this in Acero but I
fear it is a slippery slope between "a little bit of plan
manipulation" and "a full blown planner" so I've shied away from it.
My first spot to look would be a substrait-python repository which
would belong here: https://github.com/substrait-io
However, it does not appear that such a repository exists. If you're
willing to create one then a quick ask on the Substrait Slack instance
should be enough to get the repository created. Perhaps there is some
genesis of this library in Ibis although I think Ibis would use its
own representation for slicing and dicing and only use Substrait for
serialization.
Once that repository is created pyarrow could probably import it but
unless this plan manipulation makes sense purely from a pyarrow
perspective I would rather prefer that the user application import
both pyarrow and substrait-python independently.
Perhaps @Phillip Cloud or someone from the Ibis space might have some
ideas on where this might be found.
-Weston
On Thu, Jun 30, 2022 at 10:06 AM Yaron Gvili <rt...@hotmail.com>
wrote:
Hi,
Is there support for accessing Substrait protobuf Python classes
(such
as Plan) from PyArrow? If not, how should such support be added? For
example, should the PyArrow build system pull in the Substrait repo as
an
external project and build its protobuf Python classes, in a manner
similar
to how Arrow C++ does it?
I'm pondering these questions after running into an issue with code
I'm
writing under PyArrow that parses a Substrait plan represented as a
dictionary. The current (and kind of shaky) parsing operation in this
code
uses json.dumps() on the dictionary, which results in a string that is
passed to a Cython API that handles it using Arrow C++ code that has
access
to Substrait protobuf C++ classes. But when the Substrait plan
contains
a
bytes-type, json.dump() no longer works and fails with "TypeError:
Object
of type bytes is not JSON serializable". A fix for this, and a better
way
to parse, is using google.protobuf.json_format.ParseDict() [1] on the
dictionary. However, this invocation requires a second argument,
namely
a
protobuf message instance to merge with. The class of this message
(such as
Plan) is a Substrait protobuf Python class, hence the need to access
such
classes from PyArrow.
[1]
https://googleapis.dev/python/protobuf/latest/google/protobuf/json_format.html
Yaron.
