You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/06/04 03:59:23 UTC

[GitHub] [flink] leonardBang opened a new pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

leonardBang opened a new pull request #12475:
URL: https://github.com/apache/flink/pull/12475


   
   ## What is the purpose of the change
   
   *This pull request fix the wrong class loader in sql client(sqlParser).*
   
   
   ## Brief change log
   - In FLINK-17728, sql client supports parser statements via sql parser, sql parser should use user's class loader which may contains user's jar from `-j` or `-l` commands
   - Add `getExecutionContext` for Executor to expose ExecutionContext for getting user's class loader.
   
   ## Verifying this change
   
   * update existed tests and passed a e2e test in local environment.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
     - The serializers: (no)
     - The runtime per-record code paths (performance sensitive): ( no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (no)
     - If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
   


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] leonardBang commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
leonardBang commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638724618


   @dawidwys Sorry for lost my first commit, You’re right my second commit can not fix this, could you have a look  my first commit(https://github.com/apache/flink/commit/41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd) ?
   I'll push it again, I revert this the first one because they expose ExecutionContext from Executor which we think it's not a good way to fix after communicate with @godfreyhe  
   


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681) 
   * 49c19b93edb74ef702c97a4742f4936145913f35 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     }, {
       "hash" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749",
       "triggerID" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * 49c19b93edb74ef702c97a4742f4936145913f35 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717) 
   * e1ee41cc7063f83dadf380786b60733dec1639ed Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     }, {
       "hash" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749",
       "triggerID" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "57af426d3cc1db2e1a6637fc23b752daf87822ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "57af426d3cc1db2e1a6637fc23b752daf87822ed",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * e1ee41cc7063f83dadf380786b60733dec1639ed Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749) 
   * 57af426d3cc1db2e1a6637fc23b752daf87822ed UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] dawidwys commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
dawidwys commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638730649


   Sure, just let me know when it's ready for review.
   
   Honestly I also don't like too much exposing the `ExecutionContext`. 


----------------------------------------------------------------
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.

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



[GitHub] [flink] dawidwys commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
dawidwys commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638692230


   @leonardBang Have you verified the fix? Unfortunately it does not work.
   
   @godfreyhe I am not exactly what is your suggested solution, so sorry if I am wrong here. However, it is not enough to wrap instance of the parser with the `wrapClassloader`. The actual call to a method that requires the classloader must be wrapped.
   
   I think the quickest though, dirty solution would be that:
   ```
   @Override
   public Parser getSqlParser(String sessionId) {
   	final ExecutionContext<?> context = getExecutionContext(sessionId);
   	final TableEnvironment tableEnv = context.getTableEnvironment();
   	final Parser parser = ((TableEnvironmentInternal) tableEnv).getParser();
   	return new Parser() {
   		@Override
   		public List<Operation> parse(String statement) {
   			return context.wrapClassLoader(() -> parser.parse(statement));
   		}
   
   		@Override
   		public UnresolvedIdentifier parseIdentifier(String identifier) {
   			return context.wrapClassLoader(() -> parser.parseIdentifier(identifier));
   		}
   	};
   }
   ```
   
   Better solutions I could think of are:
   1. Do not instantiate the sources during parsing. I don't see a reason for that. IMO the logical plan could have just the CatalogTable, it does not necessary need to have an instantiated DynamicTableSource already. This is very demanding solution that requires lots of changes in the planner.
   2. Wrap the call to `sqlParser.parse` in `org.apache.flink.table.client.cli.SqlCommandParser`. This is tricky as we do not have access to the user classloader there.


----------------------------------------------------------------
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.

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



[GitHub] [flink] godfreyhe commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638710775


   @dawidwys For a long term, I think sql client should has an extended sql parser which can parse not not standard sql statement but also some special commands (e.g. `SOURCE`, `SET`, `REST`). which is also discussed in other prs. For now, some configs for both parser are different, such as: `SqlParserImplFactory`, `identifierMaxLength` (see: `PlannerContext#getSqlParserConfig` for blink planner and `PlanningConfigurationBuilder#getSqlParserConfig` for old planner). So it's also not clean that we copy those logic to sql client to build `org.apache.calcite.sql.parser.SqlParser`. As an interim solution, we use public interface `org.apache.flink.table.delegation.Parser` to parse a sql statement, which not only parse a statement but also validate the SqlNode (will initialize `DynamicTableSource`). 


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638588553


   Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
   to review your pull request. We will use this comment to track the progress of the review.
   
   
   ## Automated Checks
   Last check on commit 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd (Thu Jun 04 04:01:05 UTC 2020)
   
   **Warnings:**
    * No documentation files were touched! Remember to keep the Flink docs up to date!
   
   
   <sub>Mention the bot in a comment to re-run the automated checks.</sub>
   ## Review Progress
   
   * ❓ 1. The [description] looks good.
   * ❓ 2. There is [consensus] that the contribution should go into to Flink.
   * ❓ 3. Needs [attention] from.
   * ❓ 4. The change fits into the overall [architecture].
   * ❓ 5. Overall code [quality] is good.
   
   Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
    The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
    - `@flinkbot approve all` to approve all aspects
    - `@flinkbot approve-until architecture` to approve everything until `architecture`
    - `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
    - `@flinkbot disapprove architecture` to remove an approval you gave earlier
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] dawidwys edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
dawidwys edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638692230


   @leonardBang Have you verified the fix? Unfortunately it does not work.
   
   @godfreyhe I am not exactly sure what is your suggested solution, so sorry if I am wrong here. However, it is not enough to wrap instance of the parser with the `wrapClassloader`. The actual call to a method that requires the classloader must be wrapped.
   
   I think the quickest though, dirty solution would be that:
   ```
   @Override
   public Parser getSqlParser(String sessionId) {
   	final ExecutionContext<?> context = getExecutionContext(sessionId);
   	final TableEnvironment tableEnv = context.getTableEnvironment();
   	final Parser parser = ((TableEnvironmentInternal) tableEnv).getParser();
   	return new Parser() {
   		@Override
   		public List<Operation> parse(String statement) {
   			return context.wrapClassLoader(() -> parser.parse(statement));
   		}
   
   		@Override
   		public UnresolvedIdentifier parseIdentifier(String identifier) {
   			return context.wrapClassLoader(() -> parser.parseIdentifier(identifier));
   		}
   	};
   }
   ```
   
   Better solutions I could think of are:
   1. Do not instantiate the sources during parsing. I don't see a reason for that. IMO the logical plan could have just the CatalogTable, it does not necessary need to have an instantiated DynamicTableSource already. This is very demanding solution that requires lots of changes in the planner.
   2. Wrap the call to `sqlParser.parse` in `org.apache.flink.table.client.cli.SqlCommandParser`. This is tricky as we do not have access to the user classloader there.


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     }, {
       "hash" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * 49c19b93edb74ef702c97a4742f4936145913f35 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717) 
   * e1ee41cc7063f83dadf380786b60733dec1639ed UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] godfreyhe commented on a change in pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on a change in pull request #12475:
URL: https://github.com/apache/flink/pull/12475#discussion_r435663365



##########
File path: flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/gateway/local/DependencyTest.java
##########
@@ -82,6 +87,44 @@
 
 	@Test
 	public void testTableFactoryDiscovery() throws Exception {
+		final LocalExecutor executor = createExecutor();
+		final SessionContext session = new SessionContext("test-session", new Environment());
+		String sessionId = executor.openSession(session);
+		try {
+			final TableSchema result = executor.getTableSchema(sessionId, "TableNumber1");
+			final TableSchema expected = TableSchema.builder()
+				.field("IntegerField1", Types.INT())
+				.field("StringField1", Types.STRING())
+				.field("rowtimeField", Types.SQL_TIMESTAMP())
+				.build();
+
+			assertEquals(expected, result);
+		} finally {
+			executor.closeSession(sessionId);
+		}
+	}
+
+	@Test
+	public void testSqlParseWithUserClassloader() throws Exception {
+		final LocalExecutor executor = createExecutor();
+		final SessionContext session = new SessionContext("test-session", new Environment());
+		String sessionId = executor.openSession(session);
+		try {
+			String expectedPlan = "LogicalProject(IntegerField1=[$0], StringField1=[$1])\n" +
+				"  LogicalTableScan(table=[[default_catalog, default_database, TableNumber1, source: [TestTableSource]]])\n";
+			final Parser sqlParser = executor.getSqlParser(sessionId);
+			List<Operation> operations = sqlParser.parse("SELECT IntegerField1, StringField1 FROM TableNumber1");
+
+			assertTrue(operations != null && operations.size() == 1);
+			String actualPlan = RelOptUtil.toString(((PlannerQueryOperation) operations.get(0)).getCalciteTree());

Review comment:
       I think plan validation is not necessary, only checking parser is enough.




----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     }, {
       "hash" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749",
       "triggerID" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "57af426d3cc1db2e1a6637fc23b752daf87822ed",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2796",
       "triggerID" : "57af426d3cc1db2e1a6637fc23b752daf87822ed",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * 57af426d3cc1db2e1a6637fc23b752daf87822ed Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2796) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] dawidwys commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
dawidwys commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-639311897


   Thank you both, will merge once azure gives green.


----------------------------------------------------------------
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.

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



[GitHub] [flink] dawidwys closed pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
dawidwys closed pull request #12475:
URL: https://github.com/apache/flink/pull/12475


   


----------------------------------------------------------------
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.

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



[GitHub] [flink] leonardBang commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
leonardBang commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638946255


   The PR updated @dawidwys , please help take a look.


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     }, {
       "hash" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749",
       "triggerID" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "57af426d3cc1db2e1a6637fc23b752daf87822ed",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2796",
       "triggerID" : "57af426d3cc1db2e1a6637fc23b752daf87822ed",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * e1ee41cc7063f83dadf380786b60733dec1639ed Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749) 
   * 57af426d3cc1db2e1a6637fc23b752daf87822ed Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2796) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681) 
   * 49c19b93edb74ef702c97a4742f4936145913f35 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] leonardBang commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
leonardBang commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638731827


   > Sure, just let me know when it's ready for review.
   > 
   > Honestly I also don't like too much exposing the `ExecutionContext`.
   I‘ll update the PR soon


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * 49c19b93edb74ef702c97a4742f4936145913f35 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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



[GitHub] [flink] leonardBang commented on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
leonardBang commented on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-639303729


   Thanks for the review, I updated the PR @dawidwys @godfreyhe 


----------------------------------------------------------------
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.

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



[GitHub] [flink] flinkbot edited a comment on pull request #12475: [FLINK-18076][table sql / client] Sql client uses wrong class loader when parsing queries

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12475:
URL: https://github.com/apache/flink/pull/12475#issuecomment-638593958


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2681",
       "triggerID" : "fda75fa6db31dc6cc66f5b4fa7f86db3d4e26356",
       "triggerType" : "PUSH"
     }, {
       "hash" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2717",
       "triggerID" : "49c19b93edb74ef702c97a4742f4936145913f35",
       "triggerType" : "PUSH"
     }, {
       "hash" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749",
       "triggerID" : "e1ee41cc7063f83dadf380786b60733dec1639ed",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 41240cd524255ffc7cf63d2bc2ee9ff38f3abcbd UNKNOWN
   * e1ee41cc7063f83dadf380786b60733dec1639ed Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2749) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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.

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