jrsteinebrey commented on code in PR #7544:
URL: https://github.com/apache/nifi/pull/7544#discussion_r1631317574
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutDatabaseRecord.java:
##########
@@ -278,6 +283,24 @@ public class PutDatabaseRecord extends AbstractProcessor {
.defaultValue("true")
.build();
+ public static final PropertyDescriptor TRANSLATION_STRATEGY = new
PropertyDescriptor.Builder()
+ .required(true)
+ .name("Column Name Translation Strategy")
+ .description("The strategy used to normalize table column name")
Review Comment:
@ravinarayansingh The code uses all three of the these Translate properties
(Translate Field Names, Column Name Translation Strategy, and Column Name
Translation Pattern) and uses them to translate BOTH field names AND column
names. I recommend that the displayable names and descriptions be changed to
reflect the fact that they apply to both field and column names.
This comment also equally applies to UpdateDatabaseTable class.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/UpdateDatabaseTable.java:
##########
@@ -177,7 +181,25 @@ public class UpdateDatabaseTable extends AbstractProcessor
{
.allowableValues("true", "false")
.defaultValue("true")
.build();
+ public static final PropertyDescriptor TRANSLATION_STRATEGY = new
PropertyDescriptor.Builder()
+ .required(true)
+ .name("Column Name Translation Strategy")
Review Comment:
@ravinarayansingh Same comment as above that these apply to both field and
column names.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutDatabaseRecord.java:
##########
@@ -270,6 +274,7 @@ public class PutDatabaseRecord extends AbstractProcessor {
.build();
static final PropertyDescriptor TRANSLATE_FIELD_NAMES = new Builder()
+ .required(true)
.name("put-db-record-translate-field-names")
.displayName("Translate Field Names")
Review Comment:
I feel it is valuable to make these property names as clear as possible
because this is a sophisticated feature.
I think another word besides "Translate" could be found to communicate what
this feature does.
Here are some property display names I have thought of. I think Normalize is
clearer. What to people think?
"Translate Field and Column Names for Comparison"
"Normalize Field and Column Names for Comparison"
"Adjust Field and Column Names for Comparison"
"Filter Field and Column Names for Comparison"
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/db/ColumnNameNormalizer.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * 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.nifi.processors.standard.db;
+
+public interface ColumnNameNormalizer {
Review Comment:
ColumnNameNormalizer is not ideal since the class is used on json field
names and column names, but I am not sure if that is worth changing it to
something else like FieldColumnNameNormalizer or maybe NameNormalizer (maybe it
will be used in other cases besides fields or columns in the future). Thoughts?
If changed, the assoc factory class name should change also.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/db/TableSchema.java:
##########
@@ -136,14 +140,23 @@ public static TableSchema from(final Connection conn,
final String catalog, fina
} else {
// Parse the Update Keys field and normalize the column names
for (final String updateKey : updateKeys.split(",")) {
-
primaryKeyColumns.add(ColumnDescription.normalizeColumnName(updateKey.trim(),
translateColumnNames));
+ final String colName = normalizedName(updateKey,
translateColumnNames, normalizer);
+ primaryKeyColumns.add(colName);
+
}
}
- return new TableSchema(catalog, schema, tableName, cols,
translateColumnNames, primaryKeyColumns, dmd.getIdentifierQuoteString());
+ return new TableSchema(catalog, schema, tableName, cols,
translateColumnNames, normalizer, primaryKeyColumns,
dmd.getIdentifierQuoteString());
}
}
+ public static String normalizedName(final String name, final boolean
translateColumnNames, final ColumnNameNormalizer normalizer) {
+ final String colName = name.trim();
+ if (translateColumnNames && normalizer != null)
+ return normalizer.getNormalizedName(colName);
Review Comment:
I feel it would be better to put the toUpperCase() here instead of repeating
it in every normalizer.
trim() is not repeated in the normalizers and toUpperCase feels similar to
trim.
Also move trim() inside the IF check. translateNames == false should make
zero modifications to the names.
return normalizer.getNormalizedName(colName).trim().toUpperCase;
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/db/TableSchema.java:
##########
@@ -136,14 +140,23 @@ public static TableSchema from(final Connection conn,
final String catalog, fina
} else {
// Parse the Update Keys field and normalize the column names
for (final String updateKey : updateKeys.split(",")) {
-
primaryKeyColumns.add(ColumnDescription.normalizeColumnName(updateKey.trim(),
translateColumnNames));
+ final String colName = normalizedName(updateKey,
translateColumnNames, normalizer);
+ primaryKeyColumns.add(colName);
+
}
}
- return new TableSchema(catalog, schema, tableName, cols,
translateColumnNames, primaryKeyColumns, dmd.getIdentifierQuoteString());
+ return new TableSchema(catalog, schema, tableName, cols,
translateColumnNames, normalizer, primaryKeyColumns,
dmd.getIdentifierQuoteString());
}
}
+ public static String normalizedName(final String name, final boolean
translateColumnNames, final ColumnNameNormalizer normalizer) {
Review Comment:
I would remove col and column from all names in this method because field
names are also passed to this method..
translateColumnNames could be shortened to "translateNames"
colName could be "normalizedName" or "result".
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/db/TableSchema.java:
##########
@@ -136,14 +140,23 @@ public static TableSchema from(final Connection conn,
final String catalog, fina
} else {
// Parse the Update Keys field and normalize the column names
for (final String updateKey : updateKeys.split(",")) {
-
primaryKeyColumns.add(ColumnDescription.normalizeColumnName(updateKey.trim(),
translateColumnNames));
+ final String colName = normalizedName(updateKey,
translateColumnNames, normalizer);
+ primaryKeyColumns.add(colName);
+
}
}
- return new TableSchema(catalog, schema, tableName, cols,
translateColumnNames, primaryKeyColumns, dmd.getIdentifierQuoteString());
+ return new TableSchema(catalog, schema, tableName, cols,
translateColumnNames, normalizer, primaryKeyColumns,
dmd.getIdentifierQuoteString());
}
}
+ public static String normalizedName(final String name, final boolean
translateColumnNames, final ColumnNameNormalizer normalizer) {
+ final String colName = name.trim();
+ if (translateColumnNames && normalizer != null)
Review Comment:
The old normalizeName() checked for null. Old method returned a null when
null name was passed in.
Also the old method did NOT trim() the string.
Is everyone OK with those two functional code changes?
--
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]
