Skip to content

Commit

Permalink
[FLINK-35939] Do not set empty config values via ConfigUtils#encodeCo…
Browse files Browse the repository at this point in the history
…llectionToConfig
  • Loading branch information
ferenc-csaky authored Aug 19, 2024
1 parent adb8368 commit e4abd06
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ public static <IN, OUT> void encodeCollectionToConfig(
.filter(Objects::nonNull)
.collect(Collectors.toCollection(ArrayList::new));

configuration.set(key, encodedOption);
if (!encodedOption.isEmpty()) {
configuration.set(key, encodedOption);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ void nullArrayPutsNothingInConfig() {
}

@Test
void emptyCollectionPutsEmptyValueInConfig() {
void emptyCollectionPutsNothingInConfig() {
final Configuration configurationUnderTest = new Configuration();
ConfigUtils.encodeCollectionToConfig(
configurationUnderTest, TEST_OPTION, Collections.emptyList(), Object::toString);

final List<String> recovered = configurationUnderTest.get(TEST_OPTION);
assertThat(recovered).isEmpty();
assertThat(recovered).isNull();

final List<Integer> recoveredList =
ConfigUtils.decodeListFromConfig(
Expand All @@ -110,13 +110,13 @@ void emptyCollectionPutsEmptyValueInConfig() {
}

@Test
void emptyArrayPutsEmptyValueInConfig() {
void emptyArrayPutsNothingInConfig() {
final Configuration configurationUnderTest = new Configuration();
ConfigUtils.encodeArrayToConfig(
configurationUnderTest, TEST_OPTION, new Integer[5], Object::toString);

final List<String> recovered = configurationUnderTest.get(TEST_OPTION);
assertThat(recovered).isEmpty();
assertThat(recovered).isNull();

final List<Integer> recoveredList =
ConfigUtils.decodeListFromConfig(
Expand Down
20 changes: 5 additions & 15 deletions flink-table/flink-sql-client/src/test/resources/sql/set.q
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,12 @@ set;
| execution.state-recovery.ignore-unclaimed-state | false |
| execution.target | remote |
| jobmanager.rpc.address | $VAR_JOBMANAGER_RPC_ADDRESS |
| pipeline.classpaths | [] |
| pipeline.jars | [] |
| rest.port | $VAR_REST_PORT |
| sql-client.display.print-time-cost | false |
| sql-client.execution.result-mode | tableau |
| table.exec.legacy-cast-behaviour | DISABLED |
+-------------------------------------------------+-----------+
12 rows in set
10 rows in set
!ok

# reset the configuration
Expand All @@ -108,11 +106,9 @@ set;
| execution.state-recovery.ignore-unclaimed-state | false |
| execution.target | remote |
| jobmanager.rpc.address | $VAR_JOBMANAGER_RPC_ADDRESS |
| pipeline.classpaths | [] |
| pipeline.jars | [] |
| rest.port | $VAR_REST_PORT |
+-------------------------------------------------+-----------+
9 rows in set
7 rows in set
!ok

# should fail because default dialect doesn't support hive dialect
Expand Down Expand Up @@ -149,12 +145,10 @@ set;
| execution.state-recovery.ignore-unclaimed-state | false |
| execution.target | remote |
| jobmanager.rpc.address | $VAR_JOBMANAGER_RPC_ADDRESS |
| pipeline.classpaths | [] |
| pipeline.jars | [] |
| rest.port | $VAR_REST_PORT |
| sql-client.verbose | true |
+-------------------------------------------------+-----------+
10 rows in set
8 rows in set
!ok
set 'execution.attached' = 'false';
Expand All @@ -175,12 +169,10 @@ set;
| execution.state-recovery.ignore-unclaimed-state | false |
| execution.target | remote |
| jobmanager.rpc.address | $VAR_JOBMANAGER_RPC_ADDRESS |
| pipeline.classpaths | [] |
| pipeline.jars | [] |
| rest.port | $VAR_REST_PORT |
| sql-client.verbose | true |
+-------------------------------------------------+-----------+
10 rows in set
8 rows in set
!ok
# test reset can work with add jar
Expand All @@ -207,12 +199,10 @@ set;
| execution.state-recovery.ignore-unclaimed-state | false |
| execution.target | remote |
| jobmanager.rpc.address | $VAR_JOBMANAGER_RPC_ADDRESS |
| pipeline.classpaths | [] |
| pipeline.jars | [] |
| rest.port | $VAR_REST_PORT |
| sql-client.verbose | true |
+-------------------------------------------------+-----------+
10 rows in set
8 rows in set
!ok
reset;
Expand Down
8 changes: 2 additions & 6 deletions flink-table/flink-sql-gateway/src/test/resources/sql/set.q
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@ set;
| execution.state-recovery.ignore-unclaimed-state | false |
| execution.target | remote |
| jobmanager.rpc.address | localhost |
| pipeline.classpaths | [] |
| pipeline.jars | [] |
| rest.port | $VAR_REST_PORT |
+-------------------------------------------------+-----------+
9 rows in set
7 rows in set
!ok

# set illegal value
Expand All @@ -60,9 +58,7 @@ set;
| execution.state-recovery.ignore-unclaimed-state | false |
| execution.target | remote |
| jobmanager.rpc.address | localhost |
| pipeline.classpaths | [] |
| pipeline.jars | [] |
| rest.port | $VAR_REST_PORT |
+-------------------------------------------------+-----------+
9 rows in set
7 rows in set
!ok

0 comments on commit e4abd06

Please sign in to comment.