ajantha-bhat commented on pull request #4198: URL: https://github.com/apache/carbondata/pull/4198#issuecomment-898464070
@czy006 : Thanks for your interest in this. I have not reviewed PR line by line, But I do have some suggestions 1. I see that you are updating to trino 358, already 360 has released. Their release cycle is very fast. So, it will be hard to keep up. I want you to raise a discussion in dev and user mail list to see what version majority of people want to use it and do we need to support current prestosql 333 and prestodb also ? 2. Also we can't keep support for older version for long time, code becomes hard to maintain. We already have prestodb , prestosql, now trino folder. 3. Better to extract the common code (similar to we did for prestodb and prestosql) instead of duplicating all the classes. 4. Also your integration is high level integration! we need to make changes for carbon to use the new features that comes along with newer version of trino, like dynamic filtering, Rubix cache integration etc. 5. In the cluster you can test both insert flow as well as query flow to make sure functionalities are not broken, sometime unit testes are not enough (based on my experiences on presto integration) 6. Regrading your suggestion for adding JDK11 in PR builder, currently it is not possible as other modules of carbondata are not JDK11compatible yet. we get many compilation error when we compile all modules of carbondata. Example,` /repo/Carbon/format/target/gen-java/org/apache/carbondata/format/DataChunk.java:[32,24] package javax.annotation does not exist` presto 333 also needs JDK 11 for run time, so what I did is that only compile carbondata-presto jars in JDK11. For that first compile whole project in JDK8, then export JAVA_HOME path to JDK11 and run the same maven command with `-pl integration/presto` (also as I mentioned previously in https://github.com/apache/carbondata/issues/4184, use spark-2.3 profile) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@carbondata.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
