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/05/17 06:31:58 UTC
[GitHub] [flink] lsyldliu opened a new pull request, #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
lsyldliu opened a new pull request, #19742:
URL: https://github.com/apache/flink/pull/19742
## What is the purpose of the change
Support CREATE FUNCTION USING JAR syntax
## Brief change log
- *Support CREATE FUNCTION USING JAR syntax*
## Verifying this change
This change added tests and can be verified as follows:
*(example:)*
- *Added unit tests in FlinkSqlParserImplTest*
## 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, Kubernetes/Yarn, ZooKeeper: (no)
- The S3 file system connector: ( no )
## Documentation
- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (docs)
--
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] lsyldliu commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r892561081
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/resource/SqlResource.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.sql.parser.ddl.resource;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlCharStringLiteral;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.List;
+
+/** SqlNode to describe resource type and its path information. */
+public class SqlResource extends SqlCall {
Review Comment:
Currently, we only can extends it, what do you think we should extends?
--
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] wuchong commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
wuchong commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r894681266
##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -1324,15 +1324,26 @@ void testCreateFunction() {
.ok(
"CREATE TEMPORARY FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE SCALA USING JAR '/path/to/test.jar'");
+ sql("create temporary system function function1 as 'org.apache.fink.function.function1' language scala using jar '/path/to/test.jar'")
+ .ok(
+ "CREATE TEMPORARY SYSTEM FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE SCALA USING JAR '/path/to/test.jar'");
+
sql("create function function1 as 'org.apache.fink.function.function1' language java using jar 'file:///path/to/test.jar', jar 'hdfs:///path/to/test2.jar'")
.ok(
"CREATE FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE JAVA USING JAR 'file:///path/to/test.jar', JAR 'hdfs:///path/to/test2.jar'");
- sql("create temporary function function1 as 'org.apache.fink.function.function1' language sql using jar 'file:///path/to/test.jar'")
- .fails("USING JAR syntax is not applicable to SQL language.");
+ sql("create temporary function function1 as 'org.apache.fink.function.function1' language sql ^using^ jar 'file:///path/to/test.jar'")
+ .fails("CREATE FUNCTION USING JAR syntax is not applicable to SQL language.");
- sql("create temporary function function1 as 'org.apache.fink.function.function1' language python using jar 'file:///path/to/test.jar'")
- .fails("USING JAR syntax is not applicable to PYTHON language.");
+ sql("create temporary function function1 as 'org.apache.fink.function.function1' language python ^using^ jar 'file:///path/to/test.jar'")
Review Comment:
minor: the position should point to `^python^` because that's the wrong keyword.
--
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] lsyldliu commented on pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on PR #19742:
URL: https://github.com/apache/flink/pull/19742#issuecomment-1152162592
@wuchong Thanks for review, I have addresses comments.
--
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] lsyldliu commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r894986764
##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -1324,15 +1324,26 @@ void testCreateFunction() {
.ok(
"CREATE TEMPORARY FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE SCALA USING JAR '/path/to/test.jar'");
+ sql("create temporary system function function1 as 'org.apache.fink.function.function1' language scala using jar '/path/to/test.jar'")
+ .ok(
+ "CREATE TEMPORARY SYSTEM FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE SCALA USING JAR '/path/to/test.jar'");
+
sql("create function function1 as 'org.apache.fink.function.function1' language java using jar 'file:///path/to/test.jar', jar 'hdfs:///path/to/test2.jar'")
.ok(
"CREATE FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE JAVA USING JAR 'file:///path/to/test.jar', JAR 'hdfs:///path/to/test2.jar'");
- sql("create temporary function function1 as 'org.apache.fink.function.function1' language sql using jar 'file:///path/to/test.jar'")
- .fails("USING JAR syntax is not applicable to SQL language.");
+ sql("create temporary function function1 as 'org.apache.fink.function.function1' language sql ^using^ jar 'file:///path/to/test.jar'")
+ .fails("CREATE FUNCTION USING JAR syntax is not applicable to SQL language.");
- sql("create temporary function function1 as 'org.apache.fink.function.function1' language python using jar 'file:///path/to/test.jar'")
- .fails("USING JAR syntax is not applicable to PYTHON language.");
+ sql("create temporary function function1 as 'org.apache.fink.function.function1' language python ^using^ jar 'file:///path/to/test.jar'")
Review Comment:
Good catch, I have fixed 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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] luoyuxia commented on pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
luoyuxia commented on PR #19742:
URL: https://github.com/apache/flink/pull/19742#issuecomment-1150580613
@lsyldliu Thanks for updating. LGTM
--
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] luoyuxia commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
luoyuxia commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r892279638
##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -305,9 +306,52 @@ SqlCreate SqlCreateFunction(Span s, boolean replace, boolean isTemporary) :
<PYTHON> { functionLanguage = "PYTHON"; }
)
]
+ [ <USING> {
+ if ("SQL".equals(functionLanguage) || "PYTHON".equals(functionLanguage)) {
+ throw new ParseException(
+ String.format("USING JAR syntax is not applicable to %s language.", functionLanguage));
+ }
+ List<SqlNode> list = new ArrayList<SqlNode>();
Review Comment:
Would it be better use a more meaningful name instead of `list`.
--
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] lsyldliu commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r892561081
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/resource/SqlResource.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.sql.parser.ddl.resource;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlCharStringLiteral;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.List;
+
+/** SqlNode to describe resource type and its path information. */
+public class SqlResource extends SqlCall {
Review Comment:
Good catch, maybe we should extends SqlNode directly.
--
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] lsyldliu commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r893707621
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlToOperationConverterTest.java:
##########
@@ -1213,6 +1218,49 @@ public void testCreateTableWithMetadataColumn() {
assertThat(actualSchema).isEqualTo(expectedSchema);
}
+ @Test
+ public void testCreateFunction() {
+ // test create catalog function
+ String sql =
+ "CREATE FUNCTION test_udf AS 'org.apache.fink.function.function1' "
+ + "LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'";
+ final FlinkPlannerImpl planner = getPlannerBySqlDialect(SqlDialect.DEFAULT);
+ Operation operation = parse(sql, planner, getParserBySqlDialect(SqlDialect.DEFAULT));
+ assertThat(operation).isInstanceOf(CreateCatalogFunctionOperation.class);
+ CatalogFunction actualFunction =
+ ((CreateCatalogFunctionOperation) operation).getCatalogFunction();
+
+ assertThat(operation.asSummaryString())
+ .isEqualTo(
+ "CREATE CATALOG FUNCTION: (catalogFunction: [Optional[This is a user-defined function]], "
+ + "identifier: [`builtin`.`default`.`test_udf`], ignoreIfExists: [false], isTemporary: [false])");
+
+ // here doesn't assert the CatalogFunction directly because of equals method isn't
Review Comment:
remove 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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] luoyuxia commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
luoyuxia commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r892997492
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/resource/SqlResource.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.sql.parser.ddl.resource;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlCharStringLiteral;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.List;
+
+/** SqlNode to describe resource type and its path information. */
+public class SqlResource extends SqlCall {
Review Comment:
Seems all Node in Flink extend `SqlCall`.
--
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] wuchong commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
wuchong commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r893475903
##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -305,9 +306,52 @@ SqlCreate SqlCreateFunction(Span s, boolean replace, boolean isTemporary) :
<PYTHON> { functionLanguage = "PYTHON"; }
)
]
+ [ <USING> {
+ if ("SQL".equals(functionLanguage) || "PYTHON".equals(functionLanguage)) {
+ throw new ParseException(
+ String.format("USING JAR syntax is not applicable to %s language.", functionLanguage));
Review Comment:
Please use `throw SqlUtil.newContextException` to throw exception during parsing.
##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -1314,6 +1314,25 @@ void testCreateFunction() {
"CREATE SYSTEM FUNCTION is not supported, "
+ "system functions can only be registered as temporary "
+ "function, you can use CREATE TEMPORARY SYSTEM FUNCTION instead.");
+
+ // test create function using jar
+ sql("create temporary function function1 as 'org.apache.fink.function.function1' language java using jar 'file:///path/to/test.jar'")
+ .ok(
+ "CREATE TEMPORARY FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'");
+
+ sql("create temporary function function1 as 'org.apache.fink.function.function1' language scala using jar '/path/to/test.jar'")
+ .ok(
+ "CREATE TEMPORARY FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE SCALA USING JAR '/path/to/test.jar'");
Review Comment:
1. Test `CREATE TEMPORARY SYSTEM FUNCTION...`
2. Test `USING FILE ... `
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/catalog/CatalogFunction.java:
##########
@@ -57,6 +59,9 @@ public interface CatalogFunction {
/**
* Distinguish if the function is a generic function.
*
+ * @deprecated This method is currently only used in hive to determine if a function is a
Review Comment:
There is already a deprecated message below.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/resource/ResourceUri.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.resource;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+import java.util.Objects;
+
+/** Description of function resource information. */
+@PublicEvolving
+public class ResourceUri {
+
+ private final ResourceType resourceType;
+ private final String uri;
+
+ public ResourceUri(ResourceType resourceType, String uri) {
+ this.resourceType = resourceType;
+ this.uri = uri;
+ }
+
+ public ResourceType getResourceType() {
+ return resourceType;
+ }
+
+ public String getUri() {
Review Comment:
Please add Javadocs to the public methods.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/resource/ResourceUri.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.resource;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+import java.util.Objects;
+
+/** Description of function resource information. */
+@PublicEvolving
+public class ResourceUri {
+
+ private final ResourceType resourceType;
+ private final String uri;
+
+ public ResourceUri(ResourceType resourceType, String uri) {
+ this.resourceType = resourceType;
+ this.uri = uri;
+ }
+
+ public ResourceType getResourceType() {
Review Comment:
Please add Javadocs to the public methods.
##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -305,9 +306,52 @@ SqlCreate SqlCreateFunction(Span s, boolean replace, boolean isTemporary) :
<PYTHON> { functionLanguage = "PYTHON"; }
)
]
+ [ <USING> {
+ if ("SQL".equals(functionLanguage) || "PYTHON".equals(functionLanguage)) {
+ throw new ParseException(
+ String.format("USING JAR syntax is not applicable to %s language.", functionLanguage));
Review Comment:
Use the full syntax in exception message `CREATE FUNCTION USING JAR`
--
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] luoyuxia commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
luoyuxia commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r892289663
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/resource/SqlResource.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.sql.parser.ddl.resource;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlCharStringLiteral;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.List;
+
+/** SqlNode to describe resource type and its path information. */
+public class SqlResource extends SqlCall {
Review Comment:
Why `SqlResource` extends `SqlCall`, it's a little of wired to me.
--
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] wuchong merged pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
wuchong merged PR #19742:
URL: https://github.com/apache/flink/pull/19742
--
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 #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19742:
URL: https://github.com/apache/flink/pull/19742#issuecomment-1128469146
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "02cb5764f0605ffa7f41ef592e289926d7f3aa0e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "02cb5764f0605ffa7f41ef592e289926d7f3aa0e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 02cb5764f0605ffa7f41ef592e289926d7f3aa0e 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] lsyldliu commented on pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on PR #19742:
URL: https://github.com/apache/flink/pull/19742#issuecomment-1151044121
Thanks for all review, I have addressed all comments.
--
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] luoyuxia commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
luoyuxia commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r882640831
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/CatalogFunctionImpl.java:
##########
@@ -50,7 +60,7 @@ public String getClassName() {
@Override
public CatalogFunction copy() {
- return new CatalogFunctionImpl(getClassName(), functionLanguage);
+ return new CatalogFunctionImpl(getClassName(), functionLanguage, resourceUris);
Review Comment:
Is it expected that the copy isnot deep copy?
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlCreateFunction.java:
##########
@@ -51,20 +53,24 @@ public class SqlCreateFunction extends SqlCreate {
private final boolean isSystemFunction;
+ private final SqlNodeList jarPaths;
Review Comment:
I suggest to rename it for it may be a file instead of a jar, althogh currently we only support jar.
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlToOperationConverterTest.java:
##########
@@ -1213,6 +1218,54 @@ public void testCreateTableWithMetadataColumn() {
assertThat(actualSchema).isEqualTo(expectedSchema);
}
+ @Test
+ public void testCreateFunction() {
+ // test create catalog function
+ String sql =
+ "CREATE FUNCTION test_udf AS 'org.apache.fink.function.function1' "
+ + "LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'";
+ final FlinkPlannerImpl planner = getPlannerBySqlDialect(SqlDialect.DEFAULT);
+ Operation operation = parse(sql, planner, getParserBySqlDialect(SqlDialect.DEFAULT));
+ assertThat(operation).isInstanceOf(CreateCatalogFunctionOperation.class);
+ CatalogFunction actualFunction =
+ ((CreateCatalogFunctionOperation) operation).getCatalogFunction();
+
+ assertThat(operation.asSummaryString())
+ .isEqualTo(
+ "CREATE CATALOG FUNCTION: (catalogFunction: [Optional[This is a user-defined function]], "
+ + "identifier: [`builtin`.`default`.`test_udf`], ignoreIfExists: [false], isTemporary: [false])");
+
+ // here doesn't assert the CatalogFunction directly because of the isGeneric method will
+ // load the class
+ assertThat(actualFunction.getClassName()).isEqualTo("org.apache.fink.function.function1");
+ assertThat(actualFunction.getFunctionLanguage()).isEqualTo(FunctionLanguage.JAVA);
+ assertThat(actualFunction.getFunctionResources())
+ .isEqualTo(
+ Arrays.asList(
+ new ResourceUri(ResourceType.JAR, "file:///path/to/test.jar")));
+
+ // test create temporary system function
+ sql =
+ "CREATE TEMPORARY SYSTEM FUNCTION test_udf2 AS 'org.apache.fink.function.function2' "
+ + "LANGUAGE SCALA USING JAR 'file:///path/to/test.jar'";
+ operation = parse(sql, planner, getParserBySqlDialect(SqlDialect.DEFAULT));
+ assertThat(operation).isInstanceOf(CreateTempSystemFunctionOperation.class);
+ CreateTempSystemFunctionOperation tempSystemFunctionOperation =
+ (CreateTempSystemFunctionOperation) operation;
+ actualFunction = tempSystemFunctionOperation.getCatalogFunction();
+
+ // here doesn't assert the CreateTempSystemFunctionOperation directly because of the
+ // isGeneric method will load the class
+ assertThat(tempSystemFunctionOperation.getFunctionName()).isEqualTo("test_udf2");
+ assertThat(tempSystemFunctionOperation.isIgnoreIfExists()).isEqualTo(false);
+ assertThat(actualFunction.getClassName()).isEqualTo("org.apache.fink.function.function2");
+ assertThat(actualFunction.getFunctionLanguage()).isEqualTo(FunctionLanguage.SCALA);
+ assertThat(actualFunction.getFunctionResources())
+ .isEqualTo(
+ Arrays.asList(
Review Comment:
nit:
Collections.singletonList(
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlToOperationConverterTest.java:
##########
@@ -1213,6 +1218,54 @@ public void testCreateTableWithMetadataColumn() {
assertThat(actualSchema).isEqualTo(expectedSchema);
}
+ @Test
+ public void testCreateFunction() {
+ // test create catalog function
+ String sql =
+ "CREATE FUNCTION test_udf AS 'org.apache.fink.function.function1' "
+ + "LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'";
+ final FlinkPlannerImpl planner = getPlannerBySqlDialect(SqlDialect.DEFAULT);
+ Operation operation = parse(sql, planner, getParserBySqlDialect(SqlDialect.DEFAULT));
+ assertThat(operation).isInstanceOf(CreateCatalogFunctionOperation.class);
+ CatalogFunction actualFunction =
+ ((CreateCatalogFunctionOperation) operation).getCatalogFunction();
+
+ assertThat(operation.asSummaryString())
+ .isEqualTo(
+ "CREATE CATALOG FUNCTION: (catalogFunction: [Optional[This is a user-defined function]], "
+ + "identifier: [`builtin`.`default`.`test_udf`], ignoreIfExists: [false], isTemporary: [false])");
+
+ // here doesn't assert the CatalogFunction directly because of the isGeneric method will
+ // load the class
+ assertThat(actualFunction.getClassName()).isEqualTo("org.apache.fink.function.function1");
+ assertThat(actualFunction.getFunctionLanguage()).isEqualTo(FunctionLanguage.JAVA);
+ assertThat(actualFunction.getFunctionResources())
+ .isEqualTo(
+ Arrays.asList(
Review Comment:
nit:
Collections.singletonList(
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlCreateFunction.java:
##########
@@ -123,4 +139,11 @@ public String getFunctionLanguage() {
public String[] getFunctionIdentifier() {
return functionIdentifier.names.toArray(new String[0]);
}
+
+ public List<String> getJarPaths() {
Review Comment:
Also suggest to rename this method.
##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -1314,6 +1314,18 @@ void testCreateFunction() {
"CREATE SYSTEM FUNCTION is not supported, "
+ "system functions can only be registered as temporary "
+ "function, you can use CREATE TEMPORARY SYSTEM FUNCTION instead.");
+
+ // test create function using jar
+ sql("create temporary function function1 as 'org.apache.fink.function.function1' language java using jar 'file:///path/to/test.jar'")
+ .ok(
+ "CREATE TEMPORARY FUNCTION `FUNCTION1` AS 'org.apache.fink.function.function1' LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'");
+
+ sql("create function function1 as 'org.apache.fink.function.function1' language java using jar 'file:///path/to/test.jar', jar 'hdfs:///path/to/test2.jar'")
Review Comment:
add test for `LANGUAGE SCALA/PYTHON`
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/SqlToOperationConverter.java:
##########
@@ -652,19 +654,24 @@ private ModifyOperation convertAlterTableCompact(
private Operation convertCreateFunction(SqlCreateFunction sqlCreateFunction) {
UnresolvedIdentifier unresolvedIdentifier =
UnresolvedIdentifier.of(sqlCreateFunction.getFunctionIdentifier());
-
+ List<ResourceUri> resourceUris =
+ sqlCreateFunction.getJarPaths().stream()
+ .map(path -> new ResourceUri(ResourceType.JAR, path))
Review Comment:
It'll always create jar resource. It's fine when we only support jar resource. But as we want to support other resources in the future, here seems is not generic.
Some guys may struggle to modify it when them want to support other resources.
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlToOperationConverterTest.java:
##########
@@ -1213,6 +1218,54 @@ public void testCreateTableWithMetadataColumn() {
assertThat(actualSchema).isEqualTo(expectedSchema);
}
+ @Test
+ public void testCreateFunction() {
+ // test create catalog function
+ String sql =
+ "CREATE FUNCTION test_udf AS 'org.apache.fink.function.function1' "
+ + "LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'";
+ final FlinkPlannerImpl planner = getPlannerBySqlDialect(SqlDialect.DEFAULT);
+ Operation operation = parse(sql, planner, getParserBySqlDialect(SqlDialect.DEFAULT));
+ assertThat(operation).isInstanceOf(CreateCatalogFunctionOperation.class);
+ CatalogFunction actualFunction =
+ ((CreateCatalogFunctionOperation) operation).getCatalogFunction();
+
+ assertThat(operation.asSummaryString())
+ .isEqualTo(
+ "CREATE CATALOG FUNCTION: (catalogFunction: [Optional[This is a user-defined function]], "
+ + "identifier: [`builtin`.`default`.`test_udf`], ignoreIfExists: [false], isTemporary: [false])");
+
+ // here doesn't assert the CatalogFunction directly because of the isGeneric method will
+ // load the class
+ assertThat(actualFunction.getClassName()).isEqualTo("org.apache.fink.function.function1");
Review Comment:
how about extract the assert logic to a common method so that the following assert can reuse 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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] lsyldliu commented on pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on PR #19742:
URL: https://github.com/apache/flink/pull/19742#issuecomment-1149562374
@luoyuxia Thanks for your review, I have addressed your comments.
--
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] luoyuxia commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
luoyuxia commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r892282256
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlCreateFunction.java:
##########
@@ -98,6 +103,15 @@ public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
writer.keyword("LANGUAGE");
writer.keyword(functionLanguage);
}
+ if (resourceInfos.size() > 0) {
+ writer.keyword("USING");
+ SqlWriter.Frame withFrame = writer.startList("", "");
+ for (SqlNode jarPath : resourceInfos) {
Review Comment:
```suggestion
for (SqlNode resourcePath : resourceInfos) {
```
--
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] lsyldliu commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r893419950
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlToOperationConverterTest.java:
##########
@@ -1213,6 +1218,54 @@ public void testCreateTableWithMetadataColumn() {
assertThat(actualSchema).isEqualTo(expectedSchema);
}
+ @Test
+ public void testCreateFunction() {
+ // test create catalog function
+ String sql =
+ "CREATE FUNCTION test_udf AS 'org.apache.fink.function.function1' "
+ + "LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'";
+ final FlinkPlannerImpl planner = getPlannerBySqlDialect(SqlDialect.DEFAULT);
+ Operation operation = parse(sql, planner, getParserBySqlDialect(SqlDialect.DEFAULT));
+ assertThat(operation).isInstanceOf(CreateCatalogFunctionOperation.class);
+ CatalogFunction actualFunction =
+ ((CreateCatalogFunctionOperation) operation).getCatalogFunction();
+
+ assertThat(operation.asSummaryString())
+ .isEqualTo(
+ "CREATE CATALOG FUNCTION: (catalogFunction: [Optional[This is a user-defined function]], "
+ + "identifier: [`builtin`.`default`.`test_udf`], ignoreIfExists: [false], isTemporary: [false])");
+
+ // here doesn't assert the CatalogFunction directly because of the isGeneric method will
+ // load the class
+ assertThat(actualFunction.getClassName()).isEqualTo("org.apache.fink.function.function1");
Review Comment:
I think it will be better if we provide this method.
--
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] HuangZhenQiu commented on a diff in pull request #19742: [FLINK-27651][table] Support CREATE FUNCTION USING JAR syntax
Posted by GitBox <gi...@apache.org>.
HuangZhenQiu commented on code in PR #19742:
URL: https://github.com/apache/flink/pull/19742#discussion_r893103784
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlToOperationConverterTest.java:
##########
@@ -1213,6 +1218,54 @@ public void testCreateTableWithMetadataColumn() {
assertThat(actualSchema).isEqualTo(expectedSchema);
}
+ @Test
+ public void testCreateFunction() {
+ // test create catalog function
+ String sql =
+ "CREATE FUNCTION test_udf AS 'org.apache.fink.function.function1' "
+ + "LANGUAGE JAVA USING JAR 'file:///path/to/test.jar'";
+ final FlinkPlannerImpl planner = getPlannerBySqlDialect(SqlDialect.DEFAULT);
+ Operation operation = parse(sql, planner, getParserBySqlDialect(SqlDialect.DEFAULT));
+ assertThat(operation).isInstanceOf(CreateCatalogFunctionOperation.class);
+ CatalogFunction actualFunction =
+ ((CreateCatalogFunctionOperation) operation).getCatalogFunction();
+
+ assertThat(operation.asSummaryString())
+ .isEqualTo(
+ "CREATE CATALOG FUNCTION: (catalogFunction: [Optional[This is a user-defined function]], "
+ + "identifier: [`builtin`.`default`.`test_udf`], ignoreIfExists: [false], isTemporary: [false])");
+
+ // here doesn't assert the CatalogFunction directly because of the isGeneric method will
+ // load the class
+ assertThat(actualFunction.getClassName()).isEqualTo("org.apache.fink.function.function1");
Review Comment:
+1. Can we add the equal function for CatalogFunctionImpl?
--
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