[GitHub] nifi issue #2082: NIFI-2829: Fixed PutSQL time unit test.
Github user jtstorck commented on the issue: https://github.com/apache/nifi/pull/2082 +1, LGTM. Verified the three cases in the unit test, and tested with a negative long. Merging... ---
[GitHub] nifi issue #2082: NIFI-2829: Fixed PutSQL time unit test.
Github user jtstorck commented on the issue: https://github.com/apache/nifi/pull/2082 Reviewing... ---
[GitHub] nifi issue #2082: NIFI-2829: Fixed PutSQL time unit test.
Github user ijokarumawak commented on the issue: https://github.com/apache/nifi/pull/2082 @yjhyjhyjh0 Thank you very much to point that out. As you mentioned, for timezones ahead of UTC can have negative epoch values time, and we should support those are valid long values. I've updated PutSQL LONG_PATTERN to accept negative values, and also removed the use of 'GMT' from TestPutSQL completely. It works with various timezones, UTC, GMT, JST, PST and EST. @joewitt Please check when you have time. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #2082: NIFI-2829: Fixed PutSQL time unit test.
Github user yjhyjhyjh0 commented on the issue:
https://github.com/apache/nifi/pull/2082
Thanks for the detail explanation.
Indeed using local time zone is more reasonable.
Didnât notice the date will be transform into local time zone once with
value format.
Just want to point out that I first try the approach you mentioned (use
local time zone) will encounter problem with Time type.
My time zone is UTC+8, which cause time string â01:01:01â in Time type
will become negative long value.
This reasonable because Time type only consider time without epoch so it
will be negative if itâs earlier than â08:00:00â.
However this will not pass the LONG_PATTERN in PutSQL and thus consider a
negative long value to be in format "HH:mm:ss.SSSâ instead of long.
This will encounter error while parsing for sure :
java.text.ParseException: Unparseable date: â-28739000â.
If I try your commit with changing the timeStr to â01:01:01" in
testUsingDateTimeValuesWithFormatAttribute will encounter negative long parsing
problem which mentioned above.
Donât know if you will encounter similar problem with **negative long
value in Time type**, too?
I've try your commit together with LONG_PATTERN that also match negative
long value ("^-?\\d{1,19}$â) today and everything works fine now.
Thanks for any response.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #2082: NIFI-2829: Fixed PutSQL time unit test.
Github user ijokarumawak commented on the issue: https://github.com/apache/nifi/pull/2082 @joewitt The test you marked with '@Ignore' used GMT, but PutSQL and Derby database (used in unit test) treat date without timezone, i.e. local time. This caused the assertion error. I've changed the unit test from using GMT to local timezone. Confirmed the test passes with various timezone, including UTC, GMT, JST, PST, EST ... etc. Thanks for reporting the unit test failure. I believe it works fine now. Would you review this PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
