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 2022/02/10 12:57:02 UTC
[GitHub] [flink] twalthr opened a new pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
twalthr opened a new pull request #18703:
URL: https://github.com/apache/flink/pull/18703
## What is the purpose of the change
This performs a similar refactoring as done for `ContextResolvedTable`.
## Brief change log
- Introduce `ContextResolvedFunction` to cluster various information about the context of a function
- Use `ContextResolvedFunction` where appropriate
- Extend the `@PublicEvolving` `CallExpression` to contain the information of temporality of a function
## Verifying this change
This change is already covered by existing tests such as `FunctionCatalogTest`.
## 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)`: yes
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, 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? JavaDocs
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1034899976
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130",
"triggerID" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"triggerType" : "PUSH"
}, {
"hash" : "9461a982d4c2ce7f82d13b309fa2bc1766ed4c7d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "9461a982d4c2ce7f82d13b309fa2bc1766ed4c7d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130)
* 9461a982d4c2ce7f82d13b309fa2bc1766ed4c7d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1034899976
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130",
"triggerID" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1034899976
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr commented on a change in pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
twalthr commented on a change in pull request #18703:
URL: https://github.com/apache/flink/pull/18703#discussion_r805708192
##########
File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/expressions/CallExpression.java
##########
@@ -57,13 +60,13 @@
private final DataType dataType;
public CallExpression(
- FunctionIdentifier functionIdentifier,
+ boolean isTemporary,
Review comment:
I updated `CallExpression`
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1034899976
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130",
"triggerID" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"triggerType" : "PUSH"
}, {
"hash" : "ac20ca9151d221349c6894068e757575893aa926",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "ac20ca9151d221349c6894068e757575893aa926",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130)
* ac20ca9151d221349c6894068e757575893aa926 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1034894388
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 2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0 (Thu Feb 10 13:00:56 UTC 2022)
**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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr closed pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
twalthr closed pull request #18703:
URL: https://github.com/apache/flink/pull/18703
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1034899976
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130",
"triggerID" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on a change in pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
matriv commented on a change in pull request #18703:
URL: https://github.com/apache/flink/pull/18703#discussion_r805711843
##########
File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/expressions/CallExpression.java
##########
@@ -104,6 +191,11 @@ public String getFunctionName() {
}
}
+ public CallExpression replaceArgs(List<ResolvedExpression> args, DataType dataType) {
+ return new CallExpression(
+ isTemporary, functionIdentifier, functionDefinition, args, dataType);
Review comment:
nit: I would use `this.` to be 100% clear which come from this and which are replaced.
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/ContextResolvedFunction.java
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.catalog;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.expressions.CallExpression;
+import org.apache.flink.table.expressions.ResolvedExpression;
+import org.apache.flink.table.functions.BuiltInFunctionDefinition;
+import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.functions.FunctionIdentifier;
+import org.apache.flink.table.module.Module;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.util.Preconditions;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * This class contains information about a function and its relationship with a {@link Catalog}, if
+ * any.
+ *
+ * <p>There can be 3 kinds of {@link ContextResolvedFunction}:
+ *
+ * <ul>
+ * <li>A permanent function: a function which is stored in a {@link Catalog} or is provided by a
+ * {@link Module} as a system function. It always has an associated unique {@link
+ * FunctionIdentifier}.
+ * <li>A temporary table: a function which is stored in the {@link FunctionCatalog} (either as a
+ * catalog or system function), has an associated unique {@link FunctionIdentifier} and is
+ * flagged as temporary.
+ * <li>An anonymous/inline function: a function which is not stored in a catalog or module and
+ * doesn't have an associated unique {@link FunctionIdentifier}. During Table API translation,
+ * {@link BuiltInFunctionDefinition}s are treated as anonymous functions in the beginning.
+ * </ul>
+ *
+ * This class is meant for internal usages. However, it needs to be kept in sync with the public
+ * {@link CallExpression} which contains similar context information. The convenience methods {@link
+ * #fromCallExpression(CallExpression)} and {@link #toCallExpression(List, DataType)} allow a
+ * symmetric conversion.
+ */
+@Internal
+public class ContextResolvedFunction {
+
+ private final boolean isTemporary;
+
+ private final @Nullable FunctionIdentifier functionIdentifier;
+
+ private final FunctionDefinition functionDefinition;
+
+ public static ContextResolvedFunction permanent(
+ FunctionIdentifier functionIdentifier, FunctionDefinition functionDefinition) {
+ Preconditions.checkNotNull(
+ functionIdentifier,
+ "Function identifier should not be null for a permanent function.");
+ return new ContextResolvedFunction(false, functionIdentifier, functionDefinition);
+ }
+
+ public static ContextResolvedFunction temporary(
+ FunctionIdentifier functionIdentifier, FunctionDefinition functionDefinition) {
+ Preconditions.checkNotNull(
+ functionIdentifier,
+ "Function identifier should not be null for a temporary function.");
+ return new ContextResolvedFunction(true, functionIdentifier, functionDefinition);
+ }
+
+ public static ContextResolvedFunction anonymous(FunctionDefinition functionDefinition) {
+ return new ContextResolvedFunction(true, null, functionDefinition);
+ }
+
+ public static ContextResolvedFunction fromCallExpression(CallExpression callExpression) {
+ return new ContextResolvedFunction(
+ callExpression.isTemporary(),
+ callExpression.getFunctionIdentifier().orElse(null),
+ callExpression.getFunctionDefinition());
+ }
+
+ private ContextResolvedFunction(
+ boolean isTemporary,
+ @Nullable FunctionIdentifier functionIdentifier,
+ FunctionDefinition functionDefinition) {
+ this.isTemporary = isTemporary;
+ this.functionIdentifier = functionIdentifier;
+ this.functionDefinition =
+ Preconditions.checkNotNull(
+ functionDefinition, "Function definition must not be null.");
+ }
+
+ public boolean isAnonymous() {
+ return functionIdentifier == null;
+ }
+
+ /** @return true if the function is temporary. An anonymous function is always temporary. */
+ public boolean isTemporary() {
+ return isTemporary;
+ }
+
+ public boolean isPermanent() {
+ return !isTemporary;
+ }
+
+ public Optional<FunctionIdentifier> getIdentifier() {
+ return Optional.ofNullable(functionIdentifier);
+ }
+
+ public FunctionDefinition getDefinition() {
+ return functionDefinition;
+ }
+
+ public String asSummaryString() {
+ if (functionIdentifier == null) {
+ return functionDefinition.toString();
+ } else {
+ return functionIdentifier.asSummaryString();
+ }
+ }
+
+ public CallExpression toCallExpression(
Review comment:
not addressed, but also not insisting, more of a personal preference.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr commented on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
twalthr commented on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1038924152
Thanks for the feedback @matriv. I updated the PR. I think it is in a much better shape now.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr commented on a change in pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
twalthr commented on a change in pull request #18703:
URL: https://github.com/apache/flink/pull/18703#discussion_r805717145
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/ContextResolvedFunction.java
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.catalog;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.expressions.CallExpression;
+import org.apache.flink.table.expressions.ResolvedExpression;
+import org.apache.flink.table.functions.BuiltInFunctionDefinition;
+import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.functions.FunctionIdentifier;
+import org.apache.flink.table.module.Module;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.util.Preconditions;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * This class contains information about a function and its relationship with a {@link Catalog}, if
+ * any.
+ *
+ * <p>There can be 3 kinds of {@link ContextResolvedFunction}:
+ *
+ * <ul>
+ * <li>A permanent function: a function which is stored in a {@link Catalog} or is provided by a
+ * {@link Module} as a system function. It always has an associated unique {@link
+ * FunctionIdentifier}.
+ * <li>A temporary table: a function which is stored in the {@link FunctionCatalog} (either as a
+ * catalog or system function), has an associated unique {@link FunctionIdentifier} and is
+ * flagged as temporary.
+ * <li>An anonymous/inline function: a function which is not stored in a catalog or module and
+ * doesn't have an associated unique {@link FunctionIdentifier}. During Table API translation,
+ * {@link BuiltInFunctionDefinition}s are treated as anonymous functions in the beginning.
+ * </ul>
+ *
+ * This class is meant for internal usages. However, it needs to be kept in sync with the public
+ * {@link CallExpression} which contains similar context information. The convenience methods {@link
+ * #fromCallExpression(CallExpression)} and {@link #toCallExpression(List, DataType)} allow a
+ * symmetric conversion.
+ */
+@Internal
+public class ContextResolvedFunction {
+
+ private final boolean isTemporary;
+
+ private final @Nullable FunctionIdentifier functionIdentifier;
+
+ private final FunctionDefinition functionDefinition;
+
+ public static ContextResolvedFunction permanent(
+ FunctionIdentifier functionIdentifier, FunctionDefinition functionDefinition) {
+ Preconditions.checkNotNull(
+ functionIdentifier,
+ "Function identifier should not be null for a permanent function.");
+ return new ContextResolvedFunction(false, functionIdentifier, functionDefinition);
+ }
+
+ public static ContextResolvedFunction temporary(
+ FunctionIdentifier functionIdentifier, FunctionDefinition functionDefinition) {
+ Preconditions.checkNotNull(
+ functionIdentifier,
+ "Function identifier should not be null for a temporary function.");
+ return new ContextResolvedFunction(true, functionIdentifier, functionDefinition);
+ }
+
+ public static ContextResolvedFunction anonymous(FunctionDefinition functionDefinition) {
+ return new ContextResolvedFunction(true, null, functionDefinition);
+ }
+
+ public static ContextResolvedFunction fromCallExpression(CallExpression callExpression) {
+ return new ContextResolvedFunction(
+ callExpression.isTemporary(),
+ callExpression.getFunctionIdentifier().orElse(null),
+ callExpression.getFunctionDefinition());
+ }
+
+ private ContextResolvedFunction(
+ boolean isTemporary,
+ @Nullable FunctionIdentifier functionIdentifier,
+ FunctionDefinition functionDefinition) {
+ this.isTemporary = isTemporary;
+ this.functionIdentifier = functionIdentifier;
+ this.functionDefinition =
+ Preconditions.checkNotNull(
+ functionDefinition, "Function definition must not be null.");
+ }
+
+ public boolean isAnonymous() {
+ return functionIdentifier == null;
+ }
+
+ /** @return true if the function is temporary. An anonymous function is always temporary. */
+ public boolean isTemporary() {
+ return isTemporary;
+ }
+
+ public boolean isPermanent() {
+ return !isTemporary;
+ }
+
+ public Optional<FunctionIdentifier> getIdentifier() {
+ return Optional.ofNullable(functionIdentifier);
+ }
+
+ public FunctionDefinition getDefinition() {
+ return functionDefinition;
+ }
+
+ public String asSummaryString() {
+ if (functionIdentifier == null) {
+ return functionDefinition.toString();
+ } else {
+ return functionIdentifier.asSummaryString();
+ }
+ }
+
+ public CallExpression toCallExpression(
Review comment:
`fromCallExpression` close to the constructor
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on a change in pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
matriv commented on a change in pull request #18703:
URL: https://github.com/apache/flink/pull/18703#discussion_r803707057
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/ContextResolvedFunction.java
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.catalog;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.expressions.CallExpression;
+import org.apache.flink.table.expressions.ResolvedExpression;
+import org.apache.flink.table.functions.BuiltInFunctionDefinition;
+import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.functions.FunctionIdentifier;
+import org.apache.flink.table.module.Module;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.util.Preconditions;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * This class contains information about a function and its relationship with a {@link Catalog}, if
+ * any.
+ *
+ * <p>There can be 3 kinds of {@link ContextResolvedFunction}:
+ *
+ * <ul>
+ * <li>A permanent function: a function which is stored in a {@link Catalog} or is provided by a
+ * {@link Module} as a system function. It always has an associated unique {@link
+ * FunctionIdentifier}.
+ * <li>A temporary table: a function which is stored in the {@link FunctionCatalog} (either as a
Review comment:
temporary function?
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/ContextResolvedFunction.java
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.catalog;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.expressions.CallExpression;
+import org.apache.flink.table.expressions.ResolvedExpression;
+import org.apache.flink.table.functions.BuiltInFunctionDefinition;
+import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.functions.FunctionIdentifier;
+import org.apache.flink.table.module.Module;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.util.Preconditions;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * This class contains information about a function and its relationship with a {@link Catalog}, if
+ * any.
+ *
+ * <p>There can be 3 kinds of {@link ContextResolvedFunction}:
+ *
+ * <ul>
+ * <li>A permanent function: a function which is stored in a {@link Catalog} or is provided by a
+ * {@link Module} as a system function. It always has an associated unique {@link
+ * FunctionIdentifier}.
+ * <li>A temporary table: a function which is stored in the {@link FunctionCatalog} (either as a
+ * catalog or system function), has an associated unique {@link FunctionIdentifier} and is
+ * flagged as temporary.
+ * <li>An anonymous/inline function: a function which is not stored in a catalog or module and
+ * doesn't have an associated unique {@link FunctionIdentifier}. During Table API translation,
+ * {@link BuiltInFunctionDefinition}s are treated as anonymous functions in the beginning.
Review comment:
`in the beginning` is not quite clear, what happens later?
##########
File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/expressions/CallExpression.java
##########
@@ -57,13 +60,13 @@
private final DataType dataType;
public CallExpression(
- FunctionIdentifier functionIdentifier,
+ boolean isTemporary,
Review comment:
why don't create similar methods as `ContextResolvedFunction` so we callers no need to pass true/false and nulls?
##########
File path: flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/utils/SetOpRewriteUtil.scala
##########
@@ -20,9 +20,7 @@ package org.apache.flink.table.planner.plan.utils
import org.apache.flink.table.functions.BuiltInFunctionDefinitions
-import org.apache.flink.table.planner.calcite.FlinkTypeFactory.toLogicalType
Review comment:
hotfix commit maybe?
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/ContextResolvedFunction.java
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.catalog;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.expressions.CallExpression;
+import org.apache.flink.table.expressions.ResolvedExpression;
+import org.apache.flink.table.functions.BuiltInFunctionDefinition;
+import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.functions.FunctionIdentifier;
+import org.apache.flink.table.module.Module;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.util.Preconditions;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * This class contains information about a function and its relationship with a {@link Catalog}, if
+ * any.
+ *
+ * <p>There can be 3 kinds of {@link ContextResolvedFunction}:
+ *
+ * <ul>
+ * <li>A permanent function: a function which is stored in a {@link Catalog} or is provided by a
+ * {@link Module} as a system function. It always has an associated unique {@link
+ * FunctionIdentifier}.
+ * <li>A temporary table: a function which is stored in the {@link FunctionCatalog} (either as a
+ * catalog or system function), has an associated unique {@link FunctionIdentifier} and is
+ * flagged as temporary.
+ * <li>An anonymous/inline function: a function which is not stored in a catalog or module and
+ * doesn't have an associated unique {@link FunctionIdentifier}. During Table API translation,
+ * {@link BuiltInFunctionDefinition}s are treated as anonymous functions in the beginning.
+ * </ul>
+ *
+ * This class is meant for internal usages. However, it needs to be kept in sync with the public
+ * {@link CallExpression} which contains similar context information. The convenience methods {@link
+ * #fromCallExpression(CallExpression)} and {@link #toCallExpression(List, DataType)} allow a
+ * symmetric conversion.
+ */
+@Internal
+public class ContextResolvedFunction {
+
+ private final boolean isTemporary;
+
+ private final @Nullable FunctionIdentifier functionIdentifier;
+
+ private final FunctionDefinition functionDefinition;
+
+ public static ContextResolvedFunction permanent(
+ FunctionIdentifier functionIdentifier, FunctionDefinition functionDefinition) {
+ Preconditions.checkNotNull(
+ functionIdentifier,
+ "Function identifier should not be null for a permanent function.");
+ return new ContextResolvedFunction(false, functionIdentifier, functionDefinition);
+ }
+
+ public static ContextResolvedFunction temporary(
+ FunctionIdentifier functionIdentifier, FunctionDefinition functionDefinition) {
+ Preconditions.checkNotNull(
+ functionIdentifier,
+ "Function identifier should not be null for a temporary function.");
+ return new ContextResolvedFunction(true, functionIdentifier, functionDefinition);
+ }
+
+ public static ContextResolvedFunction anonymous(FunctionDefinition functionDefinition) {
+ return new ContextResolvedFunction(true, null, functionDefinition);
+ }
+
+ public static ContextResolvedFunction fromCallExpression(CallExpression callExpression) {
+ return new ContextResolvedFunction(
+ callExpression.isTemporary(),
+ callExpression.getFunctionIdentifier().orElse(null),
+ callExpression.getFunctionDefinition());
+ }
+
+ private ContextResolvedFunction(
+ boolean isTemporary,
+ @Nullable FunctionIdentifier functionIdentifier,
+ FunctionDefinition functionDefinition) {
+ this.isTemporary = isTemporary;
+ this.functionIdentifier = functionIdentifier;
+ this.functionDefinition =
+ Preconditions.checkNotNull(
+ functionDefinition, "Function definition must not be null.");
+ }
+
+ public boolean isAnonymous() {
+ return functionIdentifier == null;
+ }
+
+ /** @return true if the function is temporary. An anonymous function is always temporary. */
+ public boolean isTemporary() {
+ return isTemporary;
+ }
+
+ public boolean isPermanent() {
+ return !isTemporary;
+ }
+
+ public Optional<FunctionIdentifier> getIdentifier() {
+ return Optional.ofNullable(functionIdentifier);
+ }
+
+ public FunctionDefinition getDefinition() {
+ return functionDefinition;
+ }
+
+ public String asSummaryString() {
+ if (functionIdentifier == null) {
+ return functionDefinition.toString();
+ } else {
+ return functionIdentifier.asSummaryString();
+ }
+ }
+
+ public CallExpression toCallExpression(
Review comment:
I would keep it close to `fromCallExpression`
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr closed pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
twalthr closed pull request #18703:
URL: https://github.com/apache/flink/pull/18703
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18703: [FLINK-25870][table] Introduce ContextResolvedFunction similar to ContextResolvedTable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18703:
URL: https://github.com/apache/flink/pull/18703#issuecomment-1034899976
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130",
"triggerID" : "2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0",
"triggerType" : "PUSH"
}, {
"hash" : "ac20ca9151d221349c6894068e757575893aa926",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31402",
"triggerID" : "ac20ca9151d221349c6894068e757575893aa926",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2014ead9cb14bdaafabf8afbd7c54cc277f9a7e0 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31130)
* ac20ca9151d221349c6894068e757575893aa926 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31402)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org