Copilot commented on code in PR #1860:
URL: https://github.com/apache/fluss/pull/1860#discussion_r2485489727
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java:
##########
@@ -504,6 +514,20 @@ private boolean isDataLakeEnabled(Map<String, String>
properties) {
return Boolean.parseBoolean(dataLakeEnabledValue);
}
+ public void removeSensitiveTableOptions(Map<String, String>
tableLakeOptions) {
+ if (tableLakeOptions == null || tableLakeOptions.isEmpty()) {
+ return;
+ }
+
+ Iterator<Map.Entry<String, String>> iterator =
tableLakeOptions.entrySet().iterator();
+ while (iterator.hasNext()) {
+ String key = iterator.next().getKey().toLowerCase();
+ if (SENSITIVE_TABLE_OPTIOINS.stream().anyMatch(key::contains)) {
Review Comment:
Creating a stream from SENSITIVE_CATALOG_PROPERTIES for each map entry is
inefficient. Consider iterating over SENSITIVE_CATALOG_PROPERTIES directly
using a for-each loop or converting the stream operation to a more efficient
implementation that doesn't recreate the stream on every iteration.
```suggestion
boolean isSensitive = false;
for (String sensitiveOption : SENSITIVE_TABLE_OPTIOINS) {
if (key.contains(sensitiveOption)) {
isSensitive = true;
break;
}
}
if (isSensitive) {
```
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java:
##########
@@ -81,6 +83,14 @@ public class MetadataManager {
private final int maxBucketNum;
private final LakeCatalogDynamicLoader lakeCatalogDynamicLoader;
+ public static final Set<String> SENSITIVE_TABLE_OPTIOINS = new HashSet<>();
Review Comment:
The SENSITIVE_CATALOG_PROPERTIES set is not immutable despite being a public
static final field. Consider using `Collections.unmodifiableSet()` or Java 9+
`Set.of()` to create a truly immutable set, preventing external modification.
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java:
##########
@@ -504,6 +514,20 @@ private boolean isDataLakeEnabled(Map<String, String>
properties) {
return Boolean.parseBoolean(dataLakeEnabledValue);
}
+ public void removeSensitiveTableOptions(Map<String, String>
tableLakeOptions) {
+ if (tableLakeOptions == null || tableLakeOptions.isEmpty()) {
+ return;
+ }
+
+ Iterator<Map.Entry<String, String>> iterator =
tableLakeOptions.entrySet().iterator();
+ while (iterator.hasNext()) {
+ String key = iterator.next().getKey().toLowerCase();
+ if (SENSITIVE_TABLE_OPTIOINS.stream().anyMatch(key::contains)) {
+ iterator.remove();
+ }
+ }
Review Comment:
Bug in iterator usage: `iterator.next()` is called on line 524 to get the
key, but this consumes the iterator element. When `iterator.remove()` is called
on line 526, it attempts to remove an already-consumed element, which will
cause unexpected behavior. Store the entry from `iterator.next()` in a variable
and extract the key from it instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
