[GitHub] [incubator-superset] john-bodley opened a new pull request #8474: [SIP-15] Adding database level python-date-format

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

john-bodley opened a new pull request #8474: [SIP-15] Adding database level python-date-format
URL: https://github.com/apache/incubator-superset/pull/8474
 
 
   ### CATEGORY
   
   Choose one
   
   - [x] Bug Fix
   - [ ] Enhancement (new features, refinement)
   - [ ] Refactor
   - [ ] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   
   This PR addresses the [SIP-15](https://github.com/apache/incubator-superset/issues/6360) issue where for string column types the start endpoint may behave like `(start` rather than `[start` due to lexicographical ordering if the RHS is not encoded to mimic the formatting of the LHS. 
   
   For example at Airbnb we have over 20,000 tables in Superset which have a `ds` column (datestamp) which adheres to the `YYYY-MM-DD` ISO 8601 format however no python-date-format is specified for these columns and thus the filter endpoints behave like, 
   
   ```
   > SELECT
       '2018-01-01' >= '2018-01-01 00:00:00',
       '2018-01-01' <= '2018-01-02 00:00:00'
   FALSE TRUE
   ```
   
   i.e., `(start` rather than `[start`. This problem could be remedied by setting the python-date-format for every table but that seems overkill and doesn't ensure newly added tables will define the formatting. This PR creates a default column/python-date-format mapping on a per engine basis where one can specify a fallback (default) format if not specified. 
   
   Note https://github.com/apache/incubator-superset/pull/8464 switched the order of evaluation for obtaining the `datetime` SQL literal ensuring known per engine data types conversions, i.e., `DATE`, `TIMESTAMP`, etc. are applied prior to any python-date-format. This helps to ensure that the mapping isn't heavy handed, i.e., applying the format in an unnecessary or problematic manner. 
    
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   <img width="199" alt="Screen Shot 2019-10-29 at 11 45 32 PM" src="https://user-images.githubusercontent.com/4567245/67837195-bd12fa80-faab-11e9-9dbb-168d9383e2b3.png">
   <img width="1133" alt="Screen Shot 2019-10-29 at 11 46 04 PM" src="https://user-images.githubusercontent.com/4567245/67837196-bd12fa80-faab-11e9-84fd-7b0fc86fd317.png">
   
   ##### Before
   
   <img width="897" alt="Screen Shot 2019-10-30 at 12 11 06 AM" src="https://user-images.githubusercontent.com/4567245/67837176-ad93b180-faab-11e9-8e1c-70b89746b197.png">
   
   ##### After
   
   <img width="900" alt="Screen Shot 2019-10-30 at 12 10 54 AM" src="https://user-images.githubusercontent.com/4567245/67837175-ad93b180-faab-11e9-84a6-d065fb363038.png">
   
   
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: https://github.com/apache/incubator-superset/pull/8464
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   ### REVIEWERS
   
   to: @betodealmeida @etr2460 @michellethomas @mistercrunch @villebro @willbarrett
   

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

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org