Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/22883 )
Change subject: IMPALA-13982: Support views for Calcite planner in Impala ...................................................................... Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/22883/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22883/9//COMMIT_MSG@9 PS9, Line 9: regular views > Can you elaborate what is regular view vs non-regular view? Thanks Riza! Yes. I will elaborate more on the differences between a regular view and an inline view in the commit message and also give an example of a query that was not supported before this patch. http://gerrit.cloudera.org:8080/#/c/22883/9/java/calcite-planner/pom.xml File java/calcite-planner/pom.xml: http://gerrit.cloudera.org:8080/#/c/22883/9/java/calcite-planner/pom.xml@45 PS9, Line 45: This prevents calcite-core from using avatica-core that does not shade its : dependencies like protobuf-java during regular view expansion. > Please also mention what library what we want to avoid pulling by adding th Thanks Riza! I will try to make this a bit clearer in the next patch. I don't really want to exclude some libraries in avatica-core. It's just that avatica-core does not have the version of protobuf-java it needs on Impala process' classpath. Below are the details. According to my current understanding, during regular view expansion, Calcite would need to load the class https://github.com/apache/calcite-avatica/blob/main/core/src/main/java/org/apache/calcite/avatica/ConnectionPropertiesImpl.java, which in turn would try to make a call to a method of the class FileDescriptor with the following signature in Descriptors.java of protobuf-java, which is only available in newer versions of protobuf-java, e.g., https://github.com/protocolbuffers/protobuf/blob/v3.21.12/java/core/src/main/java/com/google/protobuf/Descriptors.java, but not in https://github.com/protocolbuffers/protobuf/blob/v2.5.0/java/src/main/java/com/google/protobuf/Descriptors.java. public static FileDescriptor internalBuildGeneratedFileFrom( final String[] descriptorDataParts, final FileDescriptor[] dependencies) Currently protobuf-java 2.5.0 is transitively pulled in by Impala's dependencies like org.apache.hadoop:hadoop-hdfs so we don't have the version of protobuf-java required by ConnectionPropertiesImpl.java on the classpath. Hence, we need an avatica artifact that has the version of protobuf-java it needs and avatica (v.s. avatica-core) is such an artifact. On a related note, I have tried explicitly adding protobuf-java 3.21.12 as a dependency of calcite-planner. This did not work since protobuf-java 2.5.0 would still be loaded by the Impala process and it's not clear to me as to how to make Impala reference protobuf-java 3.21.12 only during regular view expansion. -- To view, visit http://gerrit.cloudera.org:8080/22883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I600aae816727ae942fb221fae84c2aac63ae1893 Gerrit-Change-Number: 22883 Gerrit-PatchSet: 9 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Fri, 16 May 2025 00:54:58 +0000 Gerrit-HasComments: Yes
