Add support for Presto's REGEXP_* functions

[dongyoungy]

(Resolves #377)
Add support for the following REGEXP_* functions in Presto as shown in https://prestosql.io/docs/current/functions/regexp.html:

• REGEXP_EXTRACT_ALL
• REGEXP_EXTRACT
• REGEXP_LIKE
• REGEXP_REPLACE
• REGEXP_SPLIT

Please note that the lambda expression used in one of the example at the website is not currently supported in VerdictDB.

[pyongjoo]

Thanks! I just left minor comment regarding pom.xml

[pyongjoo]

It looks like we have been including jdbc drivers only for the test phase. Is there any difference for Presto?

[voycey]

That driver version is also very outdated, the project has also forked to prestosql/presto where the majority of the updates are being done!

…

On Mon, 10 Jun. 2019, 06:12 Yongjoo Park, ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks! I just left minor comment regarding pom.xml ------------------------------ In pom.xml <#380 (comment)>: > @@ -300,6 +300,14 @@ <activation> <jdk>1.7</jdk> </activation> + <dependencies> + <dependency> + <groupId>com.facebook.presto</groupId> + <artifactId>presto-jdbc</artifactId> + <version>0.212</version> + <!-- <scope>test</scope> --> It looks like we have been including jdbc drivers only for the test phase. Is there any difference for Presto? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#380>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIEBCUKNDD2OJUOA5OPJVLPZVP4HANCNFSM4HWIHK3A> .

[dongyoungy]

I think we had an internal discussion of 'providing JDBC drivers' vs. 'letting users use their own JDBC' and IIRC, the test-only scope has been commented out as a result. It was commented out in this commit: ac65e2c

I will update the JDBC version and restore the test scope for now.

[dongyoungy]

After a little bit of digging, it looks likes our PrestoJDBCConnection class requires PrestoStatement and QueryStats so that is actually why we removed 'test' scope previously.

[pyongjoo]

I see. I think I imported those presto-specific classes for printing out query progress.

[codecov-io]

Codecov Report

Merging #380 into master will increase coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master     #380     +/-   ##
=========================================
+ Coverage   66.43%   66.53%   +0.1%     
=========================================
  Files         169      169             
  Lines       11635    11635             
  Branches     1903     1903             
=========================================
+ Hits         7729     7740     +11     
+ Misses       3397     3387     -10     
+ Partials      509      508      -1

Impacted Files	Coverage Δ
...org/verdictdb/connection/PrestoJdbcConnection.java	36.67% <ø> (ø)	⬆️
...ain/java/org/verdictdb/sqlsyntax/PrestoSyntax.java	13.34% <100%> (ø)	⬆️
...or/QueryResultAccuracyEstimatorFromDifference.java	83.17% <0%> (-0.99%)	⬇️
...va/org/verdictdb/coordinator/ExecutionContext.java	62.13% <0%> (-0.54%)	⬇️
.../core/scrambling/FastConvergeScramblingMethod.java	91.91% <0%> (+0.58%)	⬆️
.../verdictdb/core/execplan/ExecutableNodeRunner.java	88.54% <0%> (+4.75%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 095d0d0...85f7244. Read the comment docs.

[dongyoungy]

I am merging this PR after updating the group id of presto-jdbc to io.prestosql (which should be more up-to-date).

[pyongjoo]

Thanks!

[commercial-hippie]

Thanks for this!