Add Apache Phoenix connector

[vincentpoon]

https://phoenix.apache.org/

Ref #76
Ref prestodb/presto#12111

[vincentpoon]

This is continuing from #76 since the GitHub thread there got too large

The deprecated ConnectorTableLayout usage is still there because from discussion in slack, it was determined that there isn't currently a way to get the desiredColumns during split generation.

I also added support for ARRAY type, and added a bunch of tests in TestPhoenixSqlTypeMapping

[electrum]

I'm still reviewing, but overall it's looking good. Here's the first set of comments.

[electrum]

Still reviewing. Adding another round of mostly minor comments. I still need to review PhoenixMetadata and most of the test code. Overall looking great and should be ready to merge soon.

[vincentpoon]

Is there a known issue with the builds? I'm not able to repro the test failure on my local

[electrum]

I have completed the review. This is ready to merge after the comments are addressed. Thanks for your patience and all your work on this.

[electrum]

We have been seeing spurious test failures for presto-tests and PRODUCT_TESTS_ lately. These are not related to your change.

[electrum]

The ConcurrentModificationException failure for the Phoenix tests is legitimate. Here is the problem:

• PhoenixSplit is deserialized on a worker.
  • It contains PhoenixInputSplit via WrappedPhoenixInputSplit.
• PhoenixInputSplit contains a mutable Scan object.
  • This object is modified via the PhoenixClient.getResultSet() which calls Scan.setAttribute().
• Before processing the table scan using the split, the engine calls ConnectorSplit.getInfo(), and holds onto it for later usage in logging/stats.
  • The implementation of getInfo() in JdbcSplit returns this, meaning the PhoenixSplit object.
• At some later point, in a different thread, the engine then JSON serializes the returned info object. In other words, it serializes the PhoenixSplit object.
  • The serialization code in WrappedPhoenixInputSplit calls the Hadoop Writable serialization code in PhoenixInputSplit.
  • This code ends up hitting ConcurrentModificationException, since the attributes map was modified concurrently.

I think the easiest fix is to have PhoenixClient.getResultSet() make a copy of Scan using the copy constructor before modifying it.

[vincentpoon]

Thanks for the review @electrum, I've pushed a new revision that addresses the comments, and the tests are passing now. Really appreciate all your help, I've learned a lot through the whole process.

[electrum]

Merged, thanks!

[martint]

Woohoo! Thanks @vincentpoon for your contribution!

[findepi]

🎉 🚀

[electrum]

@vincentpoon Thanks for all your hard work on this. It was a pleasure to work with you and I look forward to seeing future contributions.

[vincentpoon]

Thank you @electrum, and thanks again @findepi and @kokosing for prior reviews. The quality of the codebase speaks to the level of attention to detail in the code review process here. As a result, it's been edifying and a pleasure to work on this, and I eagerly look forward to more contributions.

FYI @lhofhansl @JamesRTaylor @aertoria

[vincentpoon]

Coincidentally, I am giving a talk at NoSql day tomorrow on this connector. Cool that I'll be able to announce that it's been merged!
https://dataworkssummit.com/nosql-day-2019/session/integrating-apache-phoenix-with-distributed-query-engines/

[electrum]

@vincentpoon Very cool! You can mention it will be available in the upcoming 312 release at https://prestosql.io/ (within the next week)

[lhofhansl]

Yeah! Thanks all!