Closed dongyoungy closed 5 years ago
This is great. Thanks!
Can you also disable Impala tests for now? I think we can focus on Presto (and possibly Spark).
@dongyoungy I see many changes for supporting catalog. But, do we need to distinguish the catalog in the first place? My understanding was that we could specify a catalog when making an initial connection to Presto; then, we can just work with schema and table names.
Merging #378 into master will decrease coverage by
0.22%
. The diff coverage is58.79%
.
@@ Coverage Diff @@
## master #378 +/- ##
==========================================
- Coverage 66.51% 66.29% -0.21%
==========================================
Files 168 169 +1
Lines 11526 11635 +109
Branches 1880 1903 +23
==========================================
+ Hits 7665 7712 +47
- Misses 3356 3411 +55
- Partials 505 512 +7
Impacted Files | Coverage Δ | |
---|---|---|
...in/java/org/verdictdb/sqlsyntax/SqlSyntaxList.java | 94.74% <ø> (ø) |
:arrow_up: |
...ain/java/org/verdictdb/sqlsyntax/PrestoSyntax.java | 13.34% <0%> (-1.3%) |
:arrow_down: |
...n/java/org/verdictdb/sqlsyntax/RedshiftSyntax.java | 5% <0%> (-0.55%) |
:arrow_down: |
.../java/org/verdictdb/connection/StaticMetaData.java | 53.71% <0%> (-6.71%) |
:arrow_down: |
.../main/java/org/verdictdb/sqlsyntax/HiveSyntax.java | 37.78% <0%> (-3.68%) |
:arrow_down: |
...g/verdictdb/connection/CachedMetaDataProvider.java | 48.58% <0%> (-8.09%) |
:arrow_down: |
...main/java/org/verdictdb/sqlsyntax/SparkSyntax.java | 60.47% <0%> (-6.2%) |
:arrow_down: |
...ain/java/org/verdictdb/sqlsyntax/SqliteSyntax.java | 12% <0%> (-0.5%) |
:arrow_down: |
...rc/main/java/org/verdictdb/sqlsyntax/H2Syntax.java | 50% <0%> (-2%) |
:arrow_down: |
...c/main/java/org/verdictdb/sqlsyntax/SqlSyntax.java | 75% <100%> (ø) |
:arrow_up: |
... and 20 more |
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 d17b2dd...ea38a68. Read the comment docs.
Feel free to merge this if you think it's ready. I'd like to rebase my branch on this.
I further fixed presto tests in pyverdict + disabled Impala unit test. I am merging this PR.
This PR makes Presto unit tests to run locally on
starburstdata/presto
docker image in build-jdk8.The original plan was to use
tpch.tiny
schema (because eventpch.sf1
was too big) for tpch queries, but it turned out that it was also too big for our CircleCI instances as Presto docker container died with exit code 137, meaning it was likely died due to OOM -- as it now usesmemory
catalog, creating scrambles in the memory during the tests --, while it worked fine locally on my machine.Therefore, I had to go back to the existing way of populating tpch data manually as before in
memory
catalog. I had to change some parameters in CircleCI config for build-jdk8 as the container still went OOM and died during the test. Now it seems to work except that it fails on some Impala tests, which has nothing to do with this PR and it just looks like our Impala test instance is not operational currently.Overall, it took some time for me to implement the 'original plan' that I had, but I had to go back to previous method, which made many of changes that I had made unnecessary :(
Nonetheless, the changes include:
PrestoMemorySyntax
for unit tests: 'memory' catalog does not support a few syntax that works for other catalogs (e.g., parititon_by)tpch.tiny
in thepresto_new
directory (ended up not using them though)