Github user gabrielreid commented on the pull request:
https://github.com/apache/incubator-phoenix/pull/26#issuecomment-41725018
A couple of points on this:
* I think that this should probably take a Timestamp as parameter instead
of a Date. Although Dates and Timestamps are currently internally equivalent in
Phoenix, this isn't the case in JDBC itself, and may change in Phoenix in the
future as well.
* I'm wondering what the rationale is for having the return value in
minutes -- I'm not totally clear on the end use case of this function, but
seeing as timestamps are typically dealt with in terms of milliseconds, it
seems as though it would likely be more useful to have a return value in
milliseconds
* It looks like there will be issues if either of the parameters is null.
The null handling for both/either of the parameters should probably be
demonstrated in the tests.
Could you explain a bit more about how you see this function being used?
I'm wondering if there is a more streamlined way of defining a function to
achieve the same end goal.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
