[GitHub] [metamodel] arjansh commented on issue #230: dev/add datatypes to excel columns

[GitBox <gi...@apache.org>]

arjansh commented on issue #230: dev/add datatypes to excel columns
URL: https://github.com/apache/metamodel/pull/230#issuecomment-535825221
 
 
   > It would have been nice to have a small discussion about this change before doing the PR because I feel like there's maybe a few things I would have liked to do differently:
   > 
   >     * The idea of detecting column type seems general enough that we should have a facility for it as a decorator pattern or such.
   
   I'm not sure how you mean that this is general, because, yes it is general because all other data stores which support data types already have their own mechanism for detecting column types, but they're all implemented in different manners, because each data store provides a different manner to get the data types. For all these data stores it's default behavior to determine the column types, so why not for Excel too?
   
   > 
   >     * The PR does not do anything to ensure conversion of values to conform with the data types. I know  we have a converter API for that in the core module.
   > 
   >     * I  do like  the change from VARCHAR to STRING. For the sake of separating concerns I would  do that in a separate PR though.
   
   > 
   > Oh, and column type detection should be _optional_. We don't want to break existing users and we also don't want it to potentially break while reading beyond the 1000th first records.
   
   I completely agree with these three last remarks. I'm also not sure if scanning a 1000 rows in an Excel file may be too much, because of possible performance issues.
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services