Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIFI-2829 Date and Time Format Support for PutSQL #1524

Closed
wants to merge 1 commit into from

Conversation

patricker
Copy link
Contributor

@patricker patricker commented Feb 21, 2017

@paulgibeault made the original PR for this, #1073.
We lost a bit of traction, had some technical issues, and it just didn't get merged.
Then #1468 came along and covered a portion of the work with a great approach. (thanks @nickcarenza)

I've gone back and updated the original code to work with the new paradigm introduced by #1468 so that it also supports DATE and TIME column types, and not just TIMESTAMP ones.

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

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

  • Has your PR been rebased against the latest commit within the target branch (typically master)?

  • 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?
  • 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?
  • If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
  • If applicable, have you updated the LICENSE file, including the main LICENSE file 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.

@patricker patricker changed the title NIFI-2829 Date and Time NIFI-2829 Date and Time Format Support for PutSQL Feb 21, 2017
final ResultSet rs = stmt.executeQuery("SELECT * FROM TIMESTAMPTEST3 ORDER BY ID");
assertTrue(rs.next());
assertEquals(1, rs.getInt(1));
assertEquals(68522000L, rs.getTime(2).getTime());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assert is failing for me, presumably because the format was ISO_LOCAL_TIME and I am at -0500 UTC. Perhaps change the constant to something that takes timezone into account?

assertTrue(rs.next());
assertEquals(1, rs.getInt(1));
assertEquals(68522000L, rs.getTime(2).getTime());
assertEquals(1012633200000L, rs.getDate(3).getTime());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same goes for this guy. Are the second and third flow files supposed to put in UTC-based dates/times? Or will they be local as well? If the latter then the same change of constants will be needed for the asserts below.

@ijokarumawak
Copy link
Member

I reviewed and confirmed #1983 addressed the unit test timezone issue. So, merged this #1524 and #1983 to master. Thanks @patricker and @yjhyjhyjh0!

@patricker patricker deleted the NIFI-2829 branch October 4, 2017 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants