ARROW-6244: [C++][Dataset] Add partition key to DataSource interface

[bkietz]

The condition is an expression guaranteed to evaluate true for all records in a DataSource. This provides some predicate push down funcitonality: DataSources whose condition precludes a filter expression will not yield any fragments (since those fragments would be filtered out anyway).

This patch does not implement evaluation of filter expressions against an in memory RecordBatch. It makes a half hearted attempt at API compatibility with #5157 which does implement this.

[fsaintjacques]

Can you dissect so we don't merge other stuff:

• Remove the RecordBatchProjector stuff
• Remove the fuse of ScanOptions/ScanContext

[bkietz]

Created https://issues.apache.org/jira/browse/ARROW-6398 for consolidating ScanContext and ScanOptions

#5239

[fsaintjacques]

@bkietz still a C++ error link error (MSCV).

[wesm]

This PR needs to be rebased.

[bkietz]

rebased

[wesm]

Can you enable Appveyor at https://ci.appveyor.com/project/bkietz/arrow?

[bkietz]

Unfortunately my appveyor username is not "bkietz" https://ci.appveyor.com/project/BenjaminKietzman/arrow/builds/27316846

[wesm]

This change gives me a working local build. Let's see what happens in Appveyor

[wesm]

Weird this issue is present with Visual Studio 2017 but not 2015

[wesm]

Well, this is awful.

Having many DLLs and needing to share symbols across them is a big problem on Windows.

I think we need to change the way we are building the library to produce a single "kitchen sink" shared library which is similar to the way that other large C++ projects do things. That shared library would also need to include the symbols current in parquet.dll, and probably also arrow_python.dll

In the meantime, I think a workaround for this mess is to make sure that all RecordBatchIterator instantiations are happening in arrow.dll. Does that seem doable?

[wesm]

cc @kou for ideas also, since this nightmarish situation also will affect our packaging

[bkietz]

That did it. @wesm @fsaintjacques what do you think?

[kou]

I don't test but can we use {} instead of = default to force using inlined implementation?

diff --git a/cpp/src/arrow/util/iterator.h b/cpp/src/arrow/util/iterator.h
index 328b8e946..8cd70f09a 100644
--- a/cpp/src/arrow/util/iterator.h
+++ b/cpp/src/arrow/util/iterator.h
@@ -32,7 +32,11 @@ namespace arrow {
 template <typename T>
 class Iterator {
  public:
-  virtual ~Iterator() = default;
+  // We can't use "= default" here to avoid using not inlined
+  // implementation with MSVC. If not inlined implementation is used,
+  // we need to export the not inlined implementation. It requires
+  // common DLL that provides the not inlined implementation.
+  virtual ~Iterator() {};
 
   /// \brief Return the next element of the sequence, nullptr when the
   /// iteration is completed

[bkietz]

@kou we tried that and a few other variations to no avail, unfortunately

[bkietz]

@pitrou here is the commit which changes Iterator from an abstract base to a type erased handle. I'll also extract to a separate pull request

[kou]

Oh, I see.

[fsaintjacques]

Travis failure is unrelated, sphinx segfault.