Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12481 )
Change subject: IMPALA-7368: Add initial support for DATE type ...................................................................... Patch Set 3: (6 comments) I had some high-level questions/comments. http://gerrit.cloudera.org:8080/#/c/12481/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12481/3//COMMIT_MSG@11 PS3, Line 11: of values supported for the DATE type is 0000-01-01 to 9999-12-31. Can you mention the literal syntax supported for date values? http://gerrit.cloudera.org:8080/#/c/12481/3//COMMIT_MSG@35 PS3, Line 35: - Implicit casting between DATE and other types: Do you plan to support the DATE 'yyyy-mm-dd' literal syntax? It looks like Hive does. I wonder if that would make it easier to write queries that depend less on implicit casting. http://gerrit.cloudera.org:8080/#/c/12481/3//COMMIT_MSG@40 PS3, Line 40: - Since both STRING -> DATE and STRING -> TIMESTAMP implicit Do you have an example of where this is required? http://gerrit.cloudera.org:8080/#/c/12481/3//COMMIT_MSG@63 PS3, Line 63: tests/query_test/test_date_queries.py. Can we add a simple test that runs a query returning date through impala-shell, just to make sure that the client-side handling of DATE there works. http://gerrit.cloudera.org:8080/#/c/12481/3//COMMIT_MSG@64 PS3, Line 64: I think we should plan on adding DATE support to the random query generator (tests/comparison), otherwise we'd be leaving an arbitrary gap in the support there. I believe the semantics of DATE should be the same for all databases so hopefully that is relatively straightforward. The coverage you have looks quite complete, and we should make efforts to understand if there are any gaps, but the extra line of defense is useful. It's also helpful if you add more standard date functions later on. http://gerrit.cloudera.org:8080/#/c/12481/3/tests/query_test/test_date_queries.py File tests/query_test/test_date_queries.py: http://gerrit.cloudera.org:8080/#/c/12481/3/tests/query_test/test_date_queries.py@18 PS3, Line 18: decimal date -- To view, visit http://gerrit.cloudera.org:8080/12481 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iea8155ef09557e0afa2f8b2d0b2dc9d0896dc30f Gerrit-Change-Number: 12481 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 14 Feb 2019 19:45:24 +0000 Gerrit-HasComments: Yes
