liukun4515 commented on PR #6873:
URL: 
https://github.com/apache/arrow-datafusion/pull/6873#issuecomment-1630295533

   > I think this PR needs some sort of test prior to merge (otherwise I think 
current_timestamp will be broken inadvertently)
   > 
   > since `current_timestamp()` seems to be an alias for `now()`` I recommend 
you either:
   > 
   > 1. Implement a rewrite in the sql planner that rewrites 
`current_timestamp` to `now()` or
   > 2. Update other places that `now()` is handled specially in the DataFusion 
codebase (like constant folding) so they also handle current_timestamp
   > 
   > Here are some places that use `now`: 
https://github.com/search?q=repo%3Aapache%2Farrow-datafusion+now
   > 
   > Given my reading of how special now is, it might make sense to explore the 
rewrite route
   
   Do you means we should rewrite the `current_timestamp` to `now()` in the 
logical phase? and delete the physical operation for the `current_timestamp`?
   
   I thinks it's a good way to maintain the new function, we just provide the 
function in the logical phase, and the function will be converted to the now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to