You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "JustinLeesin (via GitHub)" <gi...@apache.org> on 2024/02/28 03:15:06 UTC
[PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
JustinLeesin opened a new pull request, #24397:
URL: https://github.com/apache/flink/pull/24397
## What is the purpose of the change
fix[FLINK-34517](https://issues.apache.org/jira/browse/FLINK-34517)
## Brief change log
when passing StreamExecutionEnvironment in PlannerCallProcedureOperation.java , the env only contains the application-specific conf, environment-specific config not included.
As a result , environment-specific config doesn't work when executing procedure operation, which is not an expected behavior, the test case illustrates the problem.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1505948891
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/PlannerCallProcedureOperation.java:
##########
@@ -138,6 +137,14 @@ private Object[] getConvertedArgumentValues(
return argumentVal;
}
+ private ProcedureContext getProcedureContext(TableConfig tableConfig) {
+ Configuration configuration = tableConfig.getConfiguration();
Review Comment:
Yes , it's more sensible, I will change the construction order of this two conf.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1972389996
> @JustinLeesin Thanks for update. LGTM. I'll merge to master after 1.19 branch cut
@luoyuxia OK, Thank you for taking the time to review my code.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1506923378
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/ProcedureITCase.java:
##########
@@ -210,6 +214,31 @@ void testNamedArgumentsWithOptionalArguments() {
ResolvedSchema.of(Column.physical("result", DataTypes.STRING())));
}
+ @Test
+ void testEnvironmentConf() throws DatabaseAlreadyExistException {
+ // root conf should work
+ Configuration configuration = new Configuration();
+ configuration.setString("key1", "value1");
+ StreamExecutionEnvironment env =
+ StreamExecutionEnvironment.getExecutionEnvironment(configuration);
+ StreamTableEnvironment tableEnv = StreamTableEnvironment.create(env);
+ TestProcedureCatalogFactory.CatalogWithBuiltInProcedure procedureCatalog =
+ new TestProcedureCatalogFactory.CatalogWithBuiltInProcedure("procedure_catalog");
+ procedureCatalog.createDatabase(
+ "system", new CatalogDatabaseImpl(Collections.emptyMap(), null), true);
+ tableEnv.registerCatalog("test_p", procedureCatalog);
+ tableEnv.useCatalog("test_p");
Review Comment:
if also add a property with key1, it will also overwrite the same key again. Do you mean to add a property ("key2", "value22") to table config and then check?
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-2082005693
> > @JustinLeesin Could you please cherry pick it to release-1.19 branch? And if ci passed, please let me know
>
> Sorry to reply so late, I will work on it recently.
Hi, it's done on release-1.19 branch , please help to review if convenient ,PR #24656
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1507837138
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/ProcedureITCase.java:
##########
@@ -210,6 +214,31 @@ void testNamedArgumentsWithOptionalArguments() {
ResolvedSchema.of(Column.physical("result", DataTypes.STRING())));
}
+ @Test
+ void testEnvironmentConf() throws DatabaseAlreadyExistException {
+ // root conf should work
+ Configuration configuration = new Configuration();
+ configuration.setString("key1", "value1");
+ StreamExecutionEnvironment env =
+ StreamExecutionEnvironment.getExecutionEnvironment(configuration);
+ StreamTableEnvironment tableEnv = StreamTableEnvironment.create(env);
+ TestProcedureCatalogFactory.CatalogWithBuiltInProcedure procedureCatalog =
+ new TestProcedureCatalogFactory.CatalogWithBuiltInProcedure("procedure_catalog");
+ procedureCatalog.createDatabase(
+ "system", new CatalogDatabaseImpl(Collections.emptyMap(), null), true);
+ tableEnv.registerCatalog("test_p", procedureCatalog);
+ tableEnv.useCatalog("test_p");
Review Comment:
OK, I have refine the test case.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "xuyangzhong (via GitHub)" <gi...@apache.org>.
xuyangzhong commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1505886150
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/PlannerCallProcedureOperation.java:
##########
@@ -138,6 +137,14 @@ private Object[] getConvertedArgumentValues(
return argumentVal;
}
+ private ProcedureContext getProcedureContext(TableConfig tableConfig) {
+ Configuration configuration = tableConfig.getConfiguration();
Review Comment:
+1 for this comment.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1972385924
> @JustinLeesin Could you please cherry pick it to release-1.18. I'll merge to 1.18 firstly.
@luoyuxia Done , [24414](https://github.com/apache/flink/pull/24419)
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1968299984
Hi @luoyuxia , could you help to review？Thanks!
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1506889418
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/ProcedureITCase.java:
##########
@@ -210,6 +214,31 @@ void testNamedArgumentsWithOptionalArguments() {
ResolvedSchema.of(Column.physical("result", DataTypes.STRING())));
}
+ @Test
+ void testEnvironmentConf() throws DatabaseAlreadyExistException {
+ // root conf should work
+ Configuration configuration = new Configuration();
+ configuration.setString("key1", "value1");
+ StreamExecutionEnvironment env =
+ StreamExecutionEnvironment.getExecutionEnvironment(configuration);
+ StreamTableEnvironment tableEnv = StreamTableEnvironment.create(env);
+ TestProcedureCatalogFactory.CatalogWithBuiltInProcedure procedureCatalog =
+ new TestProcedureCatalogFactory.CatalogWithBuiltInProcedure("procedure_catalog");
+ procedureCatalog.createDatabase(
+ "system", new CatalogDatabaseImpl(Collections.emptyMap(), null), true);
+ tableEnv.registerCatalog("test_p", procedureCatalog);
+ tableEnv.useCatalog("test_p");
Review Comment:
Can you please also add a property ("key1, "value22") to table config , and then call the get_env_conf to make sure we can get both table configuration and root configuration
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/PlannerCallProcedureOperation.java:
##########
@@ -138,6 +137,14 @@ private Object[] getConvertedArgumentValues(
return argumentVal;
}
+ private ProcedureContext getProcedureContext(TableConfig tableConfig) {
+ Configuration configuration = (Configuration) tableConfig.getRootConfiguration();
Review Comment:
nit:
Configuration configuration = new Configuration(tableConfig.getRootConfiguration());
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1505636104
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/PlannerCallProcedureOperation.java:
##########
@@ -138,6 +137,14 @@ private Object[] getConvertedArgumentValues(
return argumentVal;
}
+ private ProcedureContext getProcedureContext(TableConfig tableConfig) {
+ Configuration configuration = tableConfig.getConfiguration();
Review Comment:
For tableConfig, it'll overwrite conf from outer root. So, won't it more reasonal the conf in tableConfig has more priority afer we reconstruct the Configuration?
I mean we should first add root config and then add conf the table config;
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/ProcedureITCase.java:
##########
@@ -210,6 +214,24 @@ void testNamedArgumentsWithOptionalArguments() {
ResolvedSchema.of(Column.physical("result", DataTypes.STRING())));
}
+ @Test
+ void testEnvironmentConf() throws DatabaseAlreadyExistException {
+ Configuration configuration = new Configuration();
+ configuration.setString("key1", "value1");
+ StreamExecutionEnvironment env =
+ StreamExecutionEnvironment.getExecutionEnvironment(configuration);
+ StreamTableEnvironment tableEnv = StreamTableEnvironment.create(env);
+ TestProcedureCatalogFactory.CatalogWithBuiltInProcedure procedureCatalog =
+ new TestProcedureCatalogFactory.CatalogWithBuiltInProcedure("procedure_catalog");
+ procedureCatalog.createDatabase(
+ "system", new CatalogDatabaseImpl(Collections.emptyMap(), null), true);
+ tableEnv.registerCatalog("test_p", procedureCatalog);
+ tableEnv.useCatalog("test_p");
Review Comment:
also set a property table confg to make sure we can also get table confg;
and set a property ("key1, "value2") to table confg to make sure the table conf overwrite the root conf
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1968982361
> Thanks for driving this. Nit: what about following the pr template like others.
OK. I have added more content in the pr , Thanks for the reminder!
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1971488431
@flinkbot run azure
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1998736348
@JustinLeesin Could you please cherry pick it to release-1.19 branch? And if ci passed, please let me know
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1505948891
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/PlannerCallProcedureOperation.java:
##########
@@ -138,6 +137,14 @@ private Object[] getConvertedArgumentValues(
return argumentVal;
}
+ private ProcedureContext getProcedureContext(TableConfig tableConfig) {
+ Configuration configuration = tableConfig.getConfiguration();
Review Comment:
Yes , it's more sensible, I will change the order.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1971516132
@flinkbot run azure
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1968123358
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8046a58c452e38f14fde0c3c9308805837f370cb",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8046a58c452e38f14fde0c3c9308805837f370cb",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8046a58c452e38f14fde0c3c9308805837f370cb 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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1507523379
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/PlannerCallProcedureOperation.java:
##########
@@ -138,6 +137,14 @@ private Object[] getConvertedArgumentValues(
return argumentVal;
}
+ private ProcedureContext getProcedureContext(TableConfig tableConfig) {
+ Configuration configuration = (Configuration) tableConfig.getRootConfiguration();
Review Comment:
Sorry for the mistake. Yes, it should be
Configuration configuration = new Configuration((Configuration)tableConfig.getRootConfiguration());
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/ProcedureITCase.java:
##########
@@ -210,6 +214,31 @@ void testNamedArgumentsWithOptionalArguments() {
ResolvedSchema.of(Column.physical("result", DataTypes.STRING())));
}
+ @Test
+ void testEnvironmentConf() throws DatabaseAlreadyExistException {
+ // root conf should work
+ Configuration configuration = new Configuration();
+ configuration.setString("key1", "value1");
+ StreamExecutionEnvironment env =
+ StreamExecutionEnvironment.getExecutionEnvironment(configuration);
+ StreamTableEnvironment tableEnv = StreamTableEnvironment.create(env);
+ TestProcedureCatalogFactory.CatalogWithBuiltInProcedure procedureCatalog =
+ new TestProcedureCatalogFactory.CatalogWithBuiltInProcedure("procedure_catalog");
+ procedureCatalog.createDatabase(
+ "system", new CatalogDatabaseImpl(Collections.emptyMap(), null), true);
+ tableEnv.registerCatalog("test_p", procedureCatalog);
+ tableEnv.useCatalog("test_p");
Review Comment:
That's right. Sorry for mistake. I mean:
you use "key1" = "value1" to create root configuration.
Then, set "key2" = "value2" to table configuration, then call the procedure.
to make sure it can get from both table configuration and root configuration
after that:
you set "key1" = "value11" to table configuration,
then call the procedure.to make sure the table configuration will overwrite root configuration
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia merged PR #24397:
URL: https://github.com/apache/flink/pull/24397
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-2082006498
> @JustinLeesin Could you please cherry pick it to release-1.19 branch? And if ci passed, please let me know
Hi, it's done on release-1.19 branch , please help to review if convenient ,PR https://github.com/apache/flink/pull/24656
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1505952061
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/ProcedureITCase.java:
##########
@@ -210,6 +214,24 @@ void testNamedArgumentsWithOptionalArguments() {
ResolvedSchema.of(Column.physical("result", DataTypes.STRING())));
}
+ @Test
+ void testEnvironmentConf() throws DatabaseAlreadyExistException {
+ Configuration configuration = new Configuration();
+ configuration.setString("key1", "value1");
+ StreamExecutionEnvironment env =
+ StreamExecutionEnvironment.getExecutionEnvironment(configuration);
+ StreamTableEnvironment tableEnv = StreamTableEnvironment.create(env);
+ TestProcedureCatalogFactory.CatalogWithBuiltInProcedure procedureCatalog =
+ new TestProcedureCatalogFactory.CatalogWithBuiltInProcedure("procedure_catalog");
+ procedureCatalog.createDatabase(
+ "system", new CatalogDatabaseImpl(Collections.emptyMap(), null), true);
+ tableEnv.registerCatalog("test_p", procedureCatalog);
+ tableEnv.useCatalog("test_p");
Review Comment:
OK, I have added some code to cover this test case.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on code in PR #24397:
URL: https://github.com/apache/flink/pull/24397#discussion_r1506925414
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/PlannerCallProcedureOperation.java:
##########
@@ -138,6 +137,14 @@ private Object[] getConvertedArgumentValues(
return argumentVal;
}
+ private ProcedureContext getProcedureContext(TableConfig tableConfig) {
+ Configuration configuration = (Configuration) tableConfig.getRootConfiguration();
Review Comment:
Yeh ,it's better, but the Configuration class doesn't provide a construct as Configuration(ReadableConfig config), it has to be : Configuration configuration = new Configuration((Configuration)tableConfig.getRootConfiguration());
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-1972271813
@JustinLeesin Could you please cherry pick it to release-1.18. I'll merge to 1.18 firstly.
--
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
Re: [PR] [FLINK-34517][table]fix environment configs ignored when calling procedure operation [flink]
Posted by "JustinLeesin (via GitHub)" <gi...@apache.org>.
JustinLeesin commented on PR #24397:
URL: https://github.com/apache/flink/pull/24397#issuecomment-2025255577
> @JustinLeesin Could you please cherry pick it to release-1.19 branch? And if ci passed, please let me know
Sorry to reply so late, I will work on it recently.
--
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