[GitHub] nifi pull request #2166: NIFI-4395 - GenerateTableFetch can't fetch column t...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/nifi/pull/2166


---

[GitHub] nifi pull request #2166: NIFI-4395 - GenerateTableFetch can't fetch column t...

[ijokarumawak]

Github user ijokarumawak commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2166#discussion_r140352593
  
--- Diff: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GenerateTableFetch.java
 ---
@@ -243,14 +243,14 @@ public void onTrigger(final ProcessContext context, 
final ProcessSessionFactory
 maxValueSelectColumns.add("MAX(" + colName + ") " + 
colName);
 String maxValue = getColumnStateMaxValue(tableName, 
statePropertyMap, colName);
 if (!StringUtils.isEmpty(maxValue)) {
-Integer type = getColumnType(tableName, colName);
+Integer type = getColumnType(context, tableName, 
colName, finalFileToProcess);
--- End diff --

Probably instead of change `getColumnType`, we can add calling `setup` if 
`columnTypeMap.isEmpty()`, before `getColumnType` here. Which makes it more 
readable.


---

[GitHub] nifi pull request #2166: NIFI-4395 - GenerateTableFetch can't fetch column t...

[ijokarumawak]

Github user ijokarumawak commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2166#discussion_r140349370
  
--- Diff: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/AbstractDatabaseFetchProcessor.java
 ---
@@ -221,8 +222,12 @@ protected PropertyDescriptor 
getSupportedDynamicPropertyDescriptor(final String
 return super.customValidate(validationContext);
 }
 
-public void setup(final ProcessContext context) {
-final String maxValueColumnNames = 
context.getProperty(MAX_VALUE_COLUMN_NAMES).evaluateAttributeExpressions().getValue();
+public void setup(final ProcessContext context){
+setup(context,true,null);
+}
+
+public void setup(final ProcessContext context, boolean 
shouldCleanCache,FlowFile flowFile) {
+final String maxValueColumnNames = (flowFile == null) ? 
context.getProperty(MAX_VALUE_COLUMN_NAMES).evaluateAttributeExpressions().getValue()
 : 
context.getProperty(MAX_VALUE_COLUMN_NAMES).evaluateAttributeExpressions(flowFile).getValue();
--- End diff --

I like the idea of being defensive, but flowFile can be null with 
`evaluateAttributeExpressions`, so we don't need this check using ternary 
operator. Just passing the flowFile (possibly null) is fine, as 
GenerateTableFetch [existing 
code](https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GenerateTableFetch.java#L185)
 does.


---

[GitHub] nifi pull request #2166: NIFI-4395 - GenerateTableFetch can't fetch column t...

[ijokarumawak]

Github user ijokarumawak commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2166#discussion_r140346314
  
--- Diff: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/AbstractDatabaseFetchProcessor.java
 ---
@@ -245,7 +250,9 @@ public void setup(final ProcessContext context) {
 ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
 int numCols = resultSetMetaData.getColumnCount();
 if (numCols > 0) {
+if (shouldCleanCache){
 columnTypeMap.clear();
+}
--- End diff --

Need indentation at the line of `columnTypeMap.clear()`.


---

[GitHub] nifi pull request #2166: NIFI-4395 - GenerateTableFetch can't fetch column t...

[ijokarumawak]

Github user ijokarumawak commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2166#discussion_r140344171
  
--- Diff: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GenerateTableFetch.java
 ---
@@ -153,13 +153,11 @@ public GenerateTableFetch() {
 
 @Override
 @OnScheduled
-public void setup(final ProcessContext context) {
-maxValueProperties = 
getDefaultMaxValueProperties(context.getProperties());
--- End diff --

We should keep this line to initialize `maxValueProperties`. I got 
following NPE when I restarted NiFi. Unit tests failed, too:

```
2017-09-22 04:55:35,898 WARN [Timer-Driven Process Thread-1] 
o.a.n.c.t.ContinuallyRunProcessorTask
java.lang.NullPointerException: null
at 
org.apache.nifi.processors.standard.GenerateTableFetch.onTrigger(GenerateTableFetch.java:208)
at 
org.apache.nifi.controller.StandardProcessorNode.onTrigger(StandardProcessorNode.java:1119)
at 
org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:147)
at 
org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:47)
at 
org.apache.nifi.controller.scheduling.TimerDrivenSchedulingAgent$1.run(TimerDrivenSchedulingAgent.java:128)
at 
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
```


---

[GitHub] nifi pull request #2166: NIFI-4395 - GenerateTableFetch can't fetch column t...

[ijokarumawak]

Github user ijokarumawak commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2166#discussion_r140341776
  
--- Diff: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GenerateTableFetch.java
 ---
@@ -402,16 +402,20 @@ private String getColumnStateMaxValue(String 
tableName, Map stat
 return maxValue;
 }
 
-private Integer getColumnType(String tableName, String colName) {
+private Integer getColumnType(final ProcessContext context, String 
tableName, String colName, FlowFile flowFile) {
 final String fullyQualifiedStateKey = getStateKey(tableName, 
colName);
 Integer type = columnTypeMap.get(fullyQualifiedStateKey);
 if (type == null && !isDynamicTableName) {
 // If the table name is static and the fully-qualified key was 
not found, try just the column name
 type = columnTypeMap.get(getStateKey(null, colName));
 }
+if (type == null || columnTypeMap.size() == 0) {
+// This means column type cache is clean after instance 
reboot. We should re-cache column type
+super.setup(context, false, flowFile);
--- End diff --

Calling `setup()` only updates `columnTypeMap`. The `type` variable will 
stay being null here. Doesn't it throw ProcessException? Shouldn't we add `type 
= columnTypeMap.get` after calling setup?


---

[GitHub] nifi pull request #2166: NIFI-4395 - GenerateTableFetch can't fetch column t...

[yjhyjhyjh0]

GitHub user yjhyjhyjh0 opened a pull request:

https://github.com/apache/nifi/pull/2166

NIFI-4395 - GenerateTableFetch can't fetch column type by state after…

… instance reboot

Thank you for submitting a contribution to Apache NiFi.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced 
 in the commit message?

- [x] Does your PR title start with NIFI- where  is the JIRA number 
you are trying to resolve? Pay particular attention to the hyphen "-" character.

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically master)?

- [x] Is your initial contribution a single, squashed commit?

### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via mvn 
-Pcontrib-check clean install at the root nifi folder?
- [x] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
- [ ] If applicable, have you updated the LICENSE file, including the main 
LICENSE file under nifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main 
NOTICE file found under nifi-assembly?
- [ ] If adding new Properties, have you added .displayName in addition to 
.name (programmatic access) for each of the new properties?

### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in 
which it is rendered?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/yjhyjhyjh0/nifi NIFI-4395

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi/pull/2166.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2166


commit faf6f2dfc15ef1900b6b868e19b90e39d934d07f
Author: Deon Huang 
Date:   2017-09-21T07:34:00Z

NIFI-4395 - GenerateTableFetch can't fetch column type by state after 
instance reboot




---