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/01/17 06:48:20 UTC
[GitHub] [flink] JingsongLi opened a new pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
JingsongLi opened a new pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879
## What is the purpose of the change
First of all, the implementation is problematic. CoreModule returns BuiltinFunctionDefinition, which cannot be resolved in FunctionCatalogOperatorTable, so it will fall back to FlinkSqlOperatorTable.
Second, the function defined by CoreModule is seriously incomplete. You can compare it with FunctionCatalogOperatorTable, a lot less. This leads to the fact that the priority of some functions is in CoreModule, and the priority of some functions is behind all modules. This is confusing, which is not what we want.
## Brief change log
Dropping the core module for now. The class CoreModule is implemented correctly but underlying layers are not ready yet.
## Verifying this change
This change is already covered by existing tests.
## 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
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575502614
<!--
Meta data
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144889679 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:add34123c391041252f5255e4e1c99f13b85f842 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4455 TriggerType:PUSH TriggerID:add34123c391041252f5255e4e1c99f13b85f842
Hash:add34123c391041252f5255e4e1c99f13b85f842 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/145091913 TriggerType:PUSH TriggerID:add34123c391041252f5255e4e1c99f13b85f842
-->
## CI report:
* dea387f141fd1d91c2082e39d9a10bab4890747d Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144889679) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421)
* add34123c391041252f5255e4e1c99f13b85f842 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/145091913) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4455)
<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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575502614
<!--
Meta data
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144889679 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
-->
## CI report:
* dea387f141fd1d91c2082e39d9a10bab4890747d Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144889679) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421)
<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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575502614
<!--
Meta data
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144889679 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:add34123c391041252f5255e4e1c99f13b85f842 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4455 TriggerType:PUSH TriggerID:add34123c391041252f5255e4e1c99f13b85f842
Hash:add34123c391041252f5255e4e1c99f13b85f842 Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/145091913 TriggerType:PUSH TriggerID:add34123c391041252f5255e4e1c99f13b85f842
-->
## CI report:
* dea387f141fd1d91c2082e39d9a10bab4890747d Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144889679) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421)
* add34123c391041252f5255e4e1c99f13b85f842 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/145091913) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4455)
<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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575498643
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 1ef1f079f2efe84d67a9e8b17921fc5ed984bbf5 (Fri Jan 17 06:51:08 UTC 2020)
**Warnings:**
* **This pull request references an unassigned [Jira ticket](https://issues.apache.org/jira/browse/FLINK-15595).** According to the [code contribution guide](https://flink.apache.org/contributing/contribute-code.html), tickets need to be assigned before starting with the implementation work.
<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
With regards,
Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575960238
Hi @bowenli86 , I am not sure for this PR. Remain unchanged is good to me too.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575502614
<!--
Meta data
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
-->
## CI report:
* dea387f141fd1d91c2082e39d9a10bab4890747d 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
With regards,
Apache Git Services
[GitHub] [flink] bowenli86 commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
bowenli86 commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368076119
##########
File path: docs/dev/table/modules.md
##########
@@ -101,8 +93,6 @@ The following types are supported out of the box.
{% highlight yaml %}
modules:
- - name: core
- type: core
Review comment:
if in 1.11 we add back core module, this would be a problem because sql client requires users listing all modules in yaml file. So if users just upgrade to 1.11 without putting core module here, sql client would assume users are explicitly not loading core.
I'm not sure what a better solution can be though, thus I'm neutral to this change
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] lirui-apache commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
lirui-apache commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368265163
##########
File path: docs/dev/table/functions/index.md
##########
@@ -84,11 +84,13 @@ The resolution order is:
1. Temporary catalog function
2. Catalog function
-## Ambiguous Function Reference
+### Ambiguous Function Reference
The resolution order is:
1. Temporary system function
-2. System function
-3. Temporary catalog function, in the current catalog and current database of the session
-4. Catalog function, in the current catalog and current database of the session
+2. Temporary catalog function, in the current catalog and current database of the session
Review comment:
I'm also wary about this change. IMHO function resolution order change can break user programs, so we probably need to have a discussion on the mailing list. Besides, I don't see how Module plays its part in the resolution. Is it treated as the `System function`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] JingsongLi commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368261218
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/FunctionCatalog.java
##########
@@ -364,14 +365,22 @@ public void dropTempCatalogFunction(ObjectIdentifier identifier, boolean ignoreI
}
Optional<FunctionDefinition> candidate = moduleManager.getFunctionDefinition(normalizedName);
- ObjectIdentifier oi = ObjectIdentifier.of(
- catalogManager.getCurrentCatalog(),
- catalogManager.getCurrentDatabase(),
- funcName);
return candidate.map(fd ->
Optional.of(new Result(FunctionIdentifier.of(funcName), fd)
- )).orElseGet(() -> resolvePreciseFunctionReference(oi));
+ )).orElseGet(() -> {
+ Optional<Result> precise = resolvePreciseFunctionReference(ObjectIdentifier.of(
+ catalogManager.getCurrentCatalog(),
+ catalogManager.getCurrentDatabase(),
+ funcName));
+ if (precise.isPresent()) {
+ return precise;
+ } else {
+ // last, fallback to core module for table api.
+ return CoreModule.INSTANCE.getFunctionDefinition(funcName)
Review comment:
Here fallback for table api. Looks not good.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-576487209
The final fix should be https://issues.apache.org/jira/browse/FLINK-15609 , but maybe it could be fix in 1.11.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575502614
<!--
Meta data
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/144889679 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
-->
## CI report:
* dea387f141fd1d91c2082e39d9a10bab4890747d Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/144889679) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421)
<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
With regards,
Apache Git Services
[GitHub] [flink] JingsongLi closed pull request #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
JingsongLi closed pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] bowenli86 commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
bowenli86 commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368074550
##########
File path: docs/dev/table/functions/index.md
##########
@@ -84,11 +84,13 @@ The resolution order is:
1. Temporary catalog function
2. Catalog function
-## Ambiguous Function Reference
+### Ambiguous Function Reference
The resolution order is:
1. Temporary system function
-2. System function
-3. Temporary catalog function, in the current catalog and current database of the session
-4. Catalog function, in the current catalog and current database of the session
+2. Temporary catalog function, in the current catalog and current database of the session
Review comment:
I think the solution should still maintain the function resolution order. Changing it would fundamentally change the behaviors, and would plant trouble for future releases.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368399847
##########
File path: docs/dev/table/modules.md
##########
@@ -101,8 +93,6 @@ The following types are supported out of the box.
{% highlight yaml %}
modules:
- - name: core
- type: core
Review comment:
This is a valid concern. Maybe we should not remove the `CoreModule` after all. I'm wondering if it would be hard to fix it instead.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-576151149
It seems that no matter how we change it, there will be some inappropriateness.
The other way is not to change any codes. Just add comment to the related document:
`CoreModule is not fully implemented now, so it does not include all built-in functions. So there are some deviations for the resolution order of built-in functions.`
What do you think?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575502614
<!--
Meta data
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144889679 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:add34123c391041252f5255e4e1c99f13b85f842 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4455 TriggerType:PUSH TriggerID:add34123c391041252f5255e4e1c99f13b85f842
Hash:add34123c391041252f5255e4e1c99f13b85f842 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/145091913 TriggerType:PUSH TriggerID:add34123c391041252f5255e4e1c99f13b85f842
-->
## CI report:
* dea387f141fd1d91c2082e39d9a10bab4890747d Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144889679) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421)
* add34123c391041252f5255e4e1c99f13b85f842 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/145091913) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4455)
<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
With regards,
Apache Git Services
[GitHub] [flink] bowenli86 commented on issue #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
bowenli86 commented on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-576389190
similar to Dawid, I wonder how hard it is to actually fix the root cause rather than trying to bypass it
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] bowenli86 commented on issue #10879: [FLINK-15595][table]
Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
bowenli86 commented on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575742781
@twalthr @dawidwys can you also take a look and share your thoughts?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368399413
##########
File path: docs/dev/table/functions/index.md
##########
@@ -84,11 +84,13 @@ The resolution order is:
1. Temporary catalog function
2. Catalog function
-## Ambiguous Function Reference
+### Ambiguous Function Reference
The resolution order is:
1. Temporary system function
-2. System function
-3. Temporary catalog function, in the current catalog and current database of the session
-4. Catalog function, in the current catalog and current database of the session
+2. Temporary catalog function, in the current catalog and current database of the session
Review comment:
I agree we should have the resolution order as defined in FLIP-57. Unfortunately as I understand the state of the implementation we have an order as follows:
1. Temporary system functions
2. Modules system functions
3. Temporary catalog functions
4. Catalog functions
5. Built-in functions
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#issuecomment-575502614
<!--
Meta data
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144889679 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:dea387f141fd1d91c2082e39d9a10bab4890747d Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421 TriggerType:PUSH TriggerID:dea387f141fd1d91c2082e39d9a10bab4890747d
Hash:add34123c391041252f5255e4e1c99f13b85f842 Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:add34123c391041252f5255e4e1c99f13b85f842
-->
## CI report:
* dea387f141fd1d91c2082e39d9a10bab4890747d Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144889679) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4421)
* add34123c391041252f5255e4e1c99f13b85f842 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
With regards,
Apache Git Services
[GitHub] [flink] bowenli86 commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
bowenli86 commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368074550
##########
File path: docs/dev/table/functions/index.md
##########
@@ -84,11 +84,13 @@ The resolution order is:
1. Temporary catalog function
2. Catalog function
-## Ambiguous Function Reference
+### Ambiguous Function Reference
The resolution order is:
1. Temporary system function
-2. System function
-3. Temporary catalog function, in the current catalog and current database of the session
-4. Catalog function, in the current catalog and current database of the session
+2. Temporary catalog function, in the current catalog and current database of the session
Review comment:
I'd prefer if we can still maintain the function resolution order defined in FLIP 57. Changing it would fundamentally change the behaviors, and would plant troubles for future releases.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] JingsongLi commented on a change in pull request #10879:
[FLINK-15595][table] Remove CoreMudule in ModuleManager
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on a change in pull request #10879: [FLINK-15595][table] Remove CoreMudule in ModuleManager
URL: https://github.com/apache/flink/pull/10879#discussion_r368407930
##########
File path: docs/dev/table/functions/index.md
##########
@@ -84,11 +84,13 @@ The resolution order is:
1. Temporary catalog function
2. Catalog function
-## Ambiguous Function Reference
+### Ambiguous Function Reference
The resolution order is:
1. Temporary system function
-2. System function
-3. Temporary catalog function, in the current catalog and current database of the session
-4. Catalog function, in the current catalog and current database of the session
+2. Temporary catalog function, in the current catalog and current database of the session
Review comment:
@dawidwys is right, actually we have three kinds of system functions:
- temporary system functions
- Module system functions: (CoreModule) built-in system functions, now it's actually planner related.
- Module system functions: user define module functions
----------------------------------------------------------------
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
With regards,
Apache Git Services