[GitHub] nifi issue #2507: Nifi 3093
Github user gardellajuanpablo commented on the issue: https://github.com/apache/nifi/pull/2507 @patricker I will create another ticket as you suggested. "Some JDBC Drivers do not support Query Timeout". ---
[GitHub] nifi issue #2507: Nifi 3093
Github user patricker commented on the issue: https://github.com/apache/nifi/pull/2507 I agree that the code is helpful, but not "Hive Support". Why not cut a new PR, something like, "Some JDBC Drivers do not support Query Timeout", and put the current fixes in there. This existing ticket could be cancelled, or rescoped, to cover just the table name issue, and any other issues found if any. ---
[GitHub] nifi issue #2507: Nifi 3093
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/2507 I'm leery of adding "Hive support" when we know there's a defect, especially when we had to fix that processor for the exact same reason (other folks were running into issues using GenerateTableFetch once it accepted multiple tables). There's also a separate thread on the mailing lists about adding the database name to the table + column name, and I think it's probably a good idea. So we wouldn't be able to support an empty table name if we add a database name, and it's a bummer to default to an empty table name when we do know how to get the table name for the Hive case. ---
[GitHub] nifi issue #2507: Nifi 3093
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/2507 I think for the setQueryTimeout() we should use the approach [here](https://github.com/apache/nifi/pull/2138/files#diff-fd2dd08c4940d5ee57f9d711ce6b426fR99). It was decided a while back that we should enforce setQueryTimeout() instead of just proceeding on if the driver doesn't support it. What the code in the other PR (#2138) does is to try to call setQueryTimeout on a Statement during customValidate, and will render the processor invalid if the user tries to set a Query Timeout to something other than zero when the driver doesn't support it. ---
[GitHub] nifi issue #2507: Nifi 3093
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2507 @markap14 @mattyb149 This looks like a simpler fix than [this one](https://github.com/apache/nifi/pull/1281). ---
[GitHub] nifi issue #2507: Nifi 3093
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2507 +1 LGTM. Didn't break anything and the changes look good. ---
