[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1238: MINIFICPP-1718 - Handle case sensitive column names

[GitBox <gi...@apache.org>]

lordgamez commented on a change in pull request #1238:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1238#discussion_r783759484



##########
File path: docker/test/integration/features/sql.feature
##########
@@ -32,6 +32,21 @@ Feature: Executing SQL operations from MiNiFi-C++
     When all instances start up
     Then at least one flowfile with the content '[{"int_col":2,"text_col":"banana"},{"int_col":1,"text_col":"apple"}]' is placed in the monitored directory in less than 120 seconds
 
+  Scenario: A MiNiFi instance can query to test table containing mixed case column names with ExecuteSQL processor
+    Given a GenerateFlowFile processor with the "File Size" property set to "0B"
+    And a UpdateAttribute processor with the "sql.args.1.value" property set to "ApPlE"
+    And the "sql.args.2.value" property of the UpdateAttribute processor is set to "BaNaNa"
+    And a ExecuteSQL processor with the "SQL select query" property set to "SELECT * FROM test_table2 WHERE "tExT_Col" = ? OR "tExT_Col" = ? ORDER BY int_col DESC"
+    And the "Output Format" property of the ExecuteSQL processor is set to "JSON"
+    And a PutFile processor with the "Directory" property set to "/tmp/output"
+    And the "success" relationship of the GenerateFlowFile processor is connected to the UpdateAttribute
+    And the "success" relationship of the UpdateAttribute processor is connected to the ExecuteSQL
+    And the "success" relationship of the ExecuteSQL processor is connected to the PutFile
+    And an ODBCService is setup up for ExecuteSQL with the name "ODBCService" and connection string "Driver={PostgreSQL ANSI};Server=postgresql-server;Port=5432;Database=postgres;Uid=postgres;Pwd=password;"

Review comment:
       This step could be refactored to have the name and the connection string hardcoded in the step implementation. I don't think it's going to change in any use case and we can remove the duplication this way.

##########
File path: docker/test/integration/features/sql.feature
##########
@@ -32,6 +32,21 @@ Feature: Executing SQL operations from MiNiFi-C++
     When all instances start up
     Then at least one flowfile with the content '[{"int_col":2,"text_col":"banana"},{"int_col":1,"text_col":"apple"}]' is placed in the monitored directory in less than 120 seconds
 
+  Scenario: A MiNiFi instance can query to test table containing mixed case column names with ExecuteSQL processor
+    Given a GenerateFlowFile processor with the "File Size" property set to "0B"
+    And a UpdateAttribute processor with the "sql.args.1.value" property set to "ApPlE"
+    And the "sql.args.2.value" property of the UpdateAttribute processor is set to "BaNaNa"
+    And a ExecuteSQL processor with the "SQL select query" property set to "SELECT * FROM test_table2 WHERE "tExT_Col" = ? OR "tExT_Col" = ? ORDER BY int_col DESC"

Review comment:
       We could add a comment that PostgreSQL's quoted columns mark case sensitive queries just to be clear.

##########
File path: extensions/sql/data/SQLIdentifier.cpp
##########
@@ -16,30 +16,26 @@
  * limitations under the License.
  */
 
-#include "Utils.h"
+#include "SQLIdentifier.h"
 
-#include <vector>
-#include <string>
+namespace org::apache::nifi::minifi::sql {
 
-#include "utils/StringUtils.h"
+SQLIdentifier::SQLIdentifier(std::string str) {

Review comment:
       As it is a self-contained entity I would add some simple unit tests for 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@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org