dawidwys edited a comment on issue #11694: [FLINK-17064][table-planner] Improve 
literals conversion in ExpressionConverter
URL: https://github.com/apache/flink/pull/11694#issuecomment-613519108
 
 
   @twalthr I updated the PR. I found it hard adding new cases in the 
`ValueLiteralExpression#getValue` so I tried to factor it out slightly to a 
more declarative approach in `LiteralConversions`. I am not entirely sure if it 
is a better approach so if you don't like this approach I can switch back to 
`if else ` statements.
   
   BTW I noticed the comment:
   >     * <p>Note to implementers: Whenever we add a new class here, make sure 
to also update the planner
   >     * for supporting the class via {@link 
CallContext#getArgumentValue(int, Class)}.
   and I am not entirely sure now what does it mean. 
   
   Does it mean that we need to support all the possible conversion classes 
meaning we need a conversion `TIME (TimeString -> Long & Int)` which are 
currently missing? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to