[GitHub] [incubator-superset] khtruong commented on a change in pull request #7625: View Presto row and array objects clearly in the data grid

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

khtruong commented on a change in pull request #7625: View Presto row and array objects clearly in the data grid
URL: https://github.com/apache/incubator-superset/pull/7625#discussion_r289224841
 
 

 ##########
 File path: tests/db_engine_specs_test.py
 ##########
 @@ -399,13 +399,127 @@ def test_presto_get_fields(self):
             self.assertEqual(actual_result.element.name, expected_result['name'])
             self.assertEqual(actual_result.name, expected_result['label'])
 
-    def test_presto_filter_presto_cols(self):
+    def test_presto_filter_out_array_nested_cols(self):
         cols = [
             {'name': 'column', 'type': 'ARRAY'},
             {'name': 'column.nested_obj', 'type': 'FLOAT'}]
-        actual_results = PrestoEngineSpec._filter_presto_cols(cols)
-        expected_results = [cols[0]]
-        self.assertEqual(actual_results, expected_results)
+        actual_filtered_cols,\
+            actual_array_cols = PrestoEngineSpec._filter_out_array_nested_cols(cols)
+        expected_filtered_cols = [{'name': 'column', 'type': 'ARRAY'}]
+        self.assertEqual(actual_filtered_cols, expected_filtered_cols)
+        self.assertEqual(actual_array_cols, cols)
+
+    def test_presto_expand_data_with_simple_structural_columns(self):
+        cols = [
+            {'name': 'row_column', 'type': 'ROW(NESTED_OBJ VARCHAR)'},
+            {'name': 'array_column', 'type': 'ARRAY(BIGINT)'}]
+        data = [
+            {'row_column': ['a'], 'array_column': [1, 2, 3]},
+            {'row_column': ['b'], 'array_column': [4, 5, 6]}]
+        actual_cols, actual_data, actual_expanded_cols = PrestoEngineSpec.expand_data(
+            cols, data)
+        expected_cols = [
+            {'name': 'row_column', 'type': 'ROW'},
 
 Review comment:
   For the first question, there are few reasons (although they may not be great reasons):
   1) type usually corresponds to class objects representing data types. We often just cast it to a string in our code base. 
   2) The string parser I created awhile ago splits the data type string based on parenthesis and comma's so we'd have to create another helper method to maintain the full data type. (This is possible if we want to).
   
   For the second question, I pass all_columns for each call to parse_structural_column to maintain order. If I concatenate the two lists selected_columns and expanded_columns, then the nested fields are not displayed right after its parent.

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