[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1738: [Bug] after flink 1.15 error when verify SQL script bug fix

[GitBox]

1996fanrui commented on code in PR #1738:
URL: 
https://github.com/apache/incubator-streampark/pull/1738#discussion_r989186820


##########
streampark-flink/streampark-flink-proxy/src/main/scala/org/apache/streampark/flink/proxy/FlinkShimsProxy.scala:
##########
@@ -88,10 +88,17 @@ object FlinkShimsProxy extends Logger {
 
     SHIMS_CLASS_LOADER_CACHE.getOrElseUpdate(s"${flinkVersion.fullVersion}", {
       // 1) flink/lib
-      val libURL = getFlinkHomeLib(flinkVersion.flinkHome)
-      val shimsUrls = ListBuffer[URL](libURL: _*)
+      def filterLib(filterLib: File): Boolean = {
+        !(filterLib.getName.startsWith("log4j") || 
filterLib.getName.startsWith("flink-table-planner-loader"))

Review Comment:
   Hi @monrg , thanks for your clarification, from your information, flink doc 
and streampark code, I got some background:
   
   1. `FlinkSqlValidator#verifySql` use 
`org.apache.flink.table.planner.parse.CalciteParser` to verify sql
   2. `org.apache.flink.table.planner.parse.CalciteParser` in the 
`flink-table-planer` module, and flink-1.15 move this module from lib to opt 
dir. ( Flink doc also describes these information[1])
   3. StreamPark add `lib/*`(exclude log4j) to the classpath during verify sql.
   
   So, `org.apache.flink.table.planner.parse.CalciteParser` cannot be found 
when verify flink-1.15 sql, right?
   
   If yes, I have some concerns:
   
   Current solution exclude `lib/flink-table-planer-loader*` and add 
`opt/flink-table-planer*` to classpath. It can solve the problem of validating 
sql, but I'm not sure whether it will introduce new problems during launch job. 
These code changes now also affect the launch flink job, right?
   
   Flink wants to import all `lib/*` when launch a job.[2] In order to verify 
the sql, streampark replaces the jar, and streampark users do not know. It 
could be a risk.
   
   So, I suggest to replace jar when only sql validation. Other processes keep 
the original logic. What do you think?
   
   <img width="1076" alt="image" 
src="https://user-images.githubusercontent.com/38427477/194349765-d381ee55-5c0d-4df5-a74b-b6daf5489784.png";>
   
   <img width="1023" alt="image" 
src="https://user-images.githubusercontent.com/38427477/194350297-eb79fd37-3e8d-4868-8c93-3a43b683275e.png";>
   
   
   [1] 
https://nightlies.apache.org/flink/flink-docs-release-1.15/docs/dev/configuration/advanced/#anatomy-of-table-dependencies
   [2] 
https://nightlies.apache.org/flink/flink-docs-release-1.15/docs/dev/configuration/advanced/#anatomy-of-the-flink-distribution
   



-- 
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: issues-unsubscr...@streampark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org