You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/02/23 11:59:55 UTC

[GitHub] [superset] yeachan153 opened a new pull request #18873: fix(sqllab/charts): casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp

yeachan153 opened a new pull request #18873:
URL: https://github.com/apache/superset/pull/18873


   ### SUMMARY
   Addresses #18871, #18596, #16487. This allows users to query date columns outside the maximum ranges of pandas timestamps: `1677-09-22 00:12:43.145225` and `2262-04-11 23:47:16.854775807`.
   
   The same fix as #14006, with a small additional fix for charts to hide the timestamps it cannot convert.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Sql Lab before:
   ![image](https://user-images.githubusercontent.com/30076090/155315294-63df3e8a-c03b-4c06-bbca-b15f1bbfc3c9.png)
   
   Sql Lab after can query past the date ranges mentioned above:
   ![image](https://user-images.githubusercontent.com/30076090/155314512-2450154e-9b73-498b-9d4c-42123a4ee4d9.png)
   
   Charts before:
   ![image](https://user-images.githubusercontent.com/30076090/155315241-23c32087-479a-44a5-80ca-740133b38e5e.png)
   
   Charts after now show data, excluding the dates that reside outside this period:
   ![image](https://user-images.githubusercontent.com/30076090/155313496-917fb149-eab0-4fca-b788-5ef87e1656fb.png)
   
   ### TESTING INSTRUCTIONS
   1. In Sql lab, try running `select TIMESTAMP any_timestamp_outside_ranges_mentioned_above`
   2. In charts, select a timeseries chart, and add a column with date ranges outside what's supported in pandas in the `TIME COLUMN`, it should still return a chart successfully and omit showing the date ranges that are unsupported.
   
   ### ADDITIONAL INFORMATION
   Fixes #18871, #18596, #16487
   - [X] Has associated issue: #18871, #18596, #16487
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [superset] villebro commented on a change in pull request #18873: fix(sqllab/charts): casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18873:
URL: https://github.com/apache/superset/pull/18873#discussion_r812941691



##########
File path: superset/result_set.py
##########
@@ -174,7 +174,13 @@ def convert_pa_dtype(pa_dtype: pa.DataType) -> Optional[str]:
 
     @staticmethod
     def convert_table_to_df(table: pa.Table) -> pd.DataFrame:
-        return table.to_pandas(integer_object_nulls=True)
+        try:
+            return table.to_pandas(integer_object_nulls=True)
+        except pa.lib.ArrowInvalid:
+            return table.to_pandas(
+                integer_object_nulls=True,
+                timestamp_as_object=True

Review comment:
       Oh, thanks for pointing that out, I didn't notice it was already enabled by default! 🤦 




-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [superset] yeachan153 commented on a change in pull request #18873: fix(sqllab/charts): casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp

Posted by GitBox <gi...@apache.org>.
yeachan153 commented on a change in pull request #18873:
URL: https://github.com/apache/superset/pull/18873#discussion_r812917892



##########
File path: superset/result_set.py
##########
@@ -174,7 +174,13 @@ def convert_pa_dtype(pa_dtype: pa.DataType) -> Optional[str]:
 
     @staticmethod
     def convert_table_to_df(table: pa.Table) -> pd.DataFrame:
-        return table.to_pandas(integer_object_nulls=True)
+        try:
+            return table.to_pandas(integer_object_nulls=True)
+        except pa.lib.ArrowInvalid:
+            return table.to_pandas(
+                integer_object_nulls=True,
+                timestamp_as_object=True

Review comment:
       It is true already by default:
   ![image](https://user-images.githubusercontent.com/30076090/155333731-63f00165-8097-4a0e-8518-cd00ea587aaf.png)
   




-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [superset] villebro commented on a change in pull request #18873: fix(sqllab/charts): casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18873:
URL: https://github.com/apache/superset/pull/18873#discussion_r812831291



##########
File path: superset/result_set.py
##########
@@ -174,7 +174,13 @@ def convert_pa_dtype(pa_dtype: pa.DataType) -> Optional[str]:
 
     @staticmethod
     def convert_table_to_df(table: pa.Table) -> pd.DataFrame:
-        return table.to_pandas(integer_object_nulls=True)
+        try:
+            return table.to_pandas(integer_object_nulls=True)
+        except pa.lib.ArrowInvalid:
+            return table.to_pandas(
+                integer_object_nulls=True,
+                timestamp_as_object=True

Review comment:
       Should we also consider `date_as_object`? https://arrow.apache.org/docs/python/generated/pyarrow.Table.html#pyarrow.Table.to_pandas




-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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