Complex data types processing

[ofekby]

Resolves part of issue #121 (resolve #121)

This pull request aims to enable the conversion of complex types support for presto.

• Cast structs (known as ROW data type in presto) to a dictionary.
• Cast the nested varbinary values from base64 encoded strings to bytes.

Note: This pull request could have been a breaking change. To avoid it the process_complex_columns parameter was added to the cursor with default value of False.

[codecov[bot]]

Codecov Report

Merging #328 (9067bdd) into master (1548ecc) will increase coverage by 0.08%. The diff coverage is 100.00%.

:exclamation: Current head 9067bdd differs from pull request most recent head c0d1a30. Consider uploading reports for the commit c0d1a30 to get more accurate results

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   93.36%   93.44%   +0.08%     
==========================================
  Files          14       14              
  Lines        1553     1572      +19     
  Branches      167      168       +1     
==========================================
+ Hits         1450     1469      +19     
  Misses         75       75              
  Partials       28       28              

Impacted Files	Coverage Δ
pyhive/presto.py	87.05% <100.00%> (+0.55%)	:arrow_up:
pyhive/tests/test_presto.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[bkyryliuk]

@ofekby did a quick pass on the diff, it looks really promising, thank you for contributing!

2 high level thoughts here, this class hierarchy is not very pythonic feels more java like. I seems like it could be rewritten via just using the functions and be in a smaller number of files.

Also could you please add integration tests as well e.g. run actual sql queries ?

[ofekby]

It might be possible to minimize the number of files, although I don't think changing it would give real profit since the code readable, already covered with unittests and it works...

Regarding the integration tests - I will add a few in the next days

[ofekby]

• Added an integration test with complex new table.
• Fixed some issues with the map processing (casting map keys to corresponding python type)

If you @bkyryliuk could please review or assign someone to review, thanks.

[kakipipi23]

Wow this PR is a game changer for the whole Presto community! Thanks!

[ofekby]

• Removed the class inheritance
• Changed the processors to be a functions

@bkyryliuk please provide more detailed comments if there is any issue with the code

[boazberman]

Much needed and appreciated! Thanks!

[orstav]

Wow thats super important feature can't wait until it get merged

[yana33083]

Thank you so much for adding this!

[ilanb1996]

Waiting for this for 6 months!!!

[GalDayan]

Looks great, thank you!

[boazberman]

Any updates?

[hashhar]

@bkyryliuk Is there anything the community can do to help push this over the finish line? We need this and I really want to avoid maintaining an internal fork with this patch applied.

[boazberman]

This is super important, isn't there anything that can be done to prompt the merge request? @bkyryliuk

[GalDayan]

@bkyryliuk Any updates?

[panoptikum]

+1. Thanks for the contribution

[bkyryliuk]

@ofekby please make sure that https://github.com/ofekby/presto-types-parser is py2 friendly @hashhar if you can test it in your dev / prod env - it would be great, I don't have capacity to do it right now

[bkyryliuk]

I am on parental leave now, @ashishgandhi / @m-kogan could someone take a look ?

[CLAassistant]

All committers have signed the CLA.

[ofekby]

I am on parental leave now, @ashishgandhi / @m-kogan could someone take a look ?

@ashishgandhi / @m-kogan if you could please review the changes, and enable back the CI process for this PR.

[ottomata]

What's the status here? We are waiting for this at the Wikimedia Foundation! :)