cccs-RyanS commented on pull request #18794:
URL: https://github.com/apache/superset/pull/18794#issuecomment-1055695929


   Hey @zhaoyongjie thanks for the comment, I'll give you my thoughts!
   
     1.  So business types were proposed more to define custom behaviour that 
you may want to attach to a column like validation, display and conversion of 
values that might not be super straightforward such as dot decimal IP to an 
integer. We also want to allow multiple types of input, moving forward with IP 
as an example one user might want to type in 1.1.1.1 even though the IP is 
sorted as an integer another user may want to type in 16843009 on the same 
column. So we want to be able to define a type to handle that.  
     
     2. Yes this PR only covers the input / conversions for filters. Follow up 
work will need to be done in order to display the converted values that are 
returned from the query. I agree this does mean the current state is a bit 
inconsistent. If it is preferred to implement more of the functionality at once 
I can do that 
     
     3.  I agree this does make sense for the port type. The only things to 
note is we would like to indicate what values something like ftp represents to 
the user as ftp could potentially mean just 21 but we included 20 as well. we 
would also like to preserve the ability for a user to simply type the numeric 
version of the port.
     
     4.  Yea extending the operators would help mitigate that issue, however 
would that result in a lot of operators as there are quite a few valid cidr 
block options 
     
     I'm not sure if that answered any of your questions, but let me know! I'd 
be happy to try to elaborate more.


-- 
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to