[GitHub] [arrow] westonpace commented on pull request #13355: ARROW-16796: [C++] Fix bad defaulting of ExecContext argument

[GitBox <gi...@apache.org>]

westonpace commented on PR #13355:
URL: https://github.com/apache/arrow/pull/13355#issuecomment-1151679039

   Good catch.  I wonder if we should remove the default argument to bind entirely (it would look something like https://github.com/westonpace/arrow/commit/c9ae1dd6a0857af69e48a95ec76480f4c466791e ).  Looks like there are only a few other non-test spots we call bind.
   
    * In the parquet reader we convert statistics into expressions and bind them to the schema.  These expressions will only use min/max and it's only really for simplification and not execution so we're probably ok.
    * The scanner has a number of methods that create exec plans (this is the "lightweight producer" half of the scanner).  We could arguably add an ExecContext to scan options but I think it would better to start phasing out this half of the scanner in favor of direct use of exec plans instead.
   
   CC @bkietz for second opinion


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org