feat: Visualize complex Hive column types v2 #1091

baumandm · 2022-12-01T22:07:13Z

This is an alternate version of #1072, replacing the JSON viewer with a more natural-looking nested type drilldown. I've also addressed some issues with the original version.

Compared to #1072, the parser is more consistent and does a better job of handling certain types than before. To support the nesting, it always creates nodes with optional children. Now array<> and uniontype<> types will create placeholder <element> nodes underneath them, and map<> types create placeholder <key> and <value> nodes.

Complex type parsing is case-insensitive (e.g. struct vs STRUCT) but otherwise case is preserved.

There are three main features:

Columns with complex types can be expanded directly in the TablePanelView to see a nested representation (initially collapsed).

The type displayed on the ColumnPanelView is pretty-printed with simple line breaks/indents.

The DataTableColumnCard shows a similar nested visualization after expanding a column (initially collapsed).

baumandm · 2022-12-02T16:27:49Z

I've extended the length of DataTableColumn's type column to 1024 characters, since we were running into a number of truncated columns.

In the event that a column type is still longer than the new limit, everything degrades gracefully and the partial type string will be short as before.

jczhong84

Looks good to me overall, thanks for the update!

Also could you update the version in package.json?

jczhong84 · 2022-12-14T21:22:42Z

querybook/webapp/lib/utils/complex-types.ts

+    if (!matches || matches.length < 3) {
+        return { key, type };
+    }


is this to handle the truncated case?

could you add a test case for it?

what if the truncated type also matches the regex? like struct<date:struct<hour:int>

Added a case for non-matching truncated, and the 2nd scenario where the truncated type matches the regex accidentally. I've addressed this by checking the depth field to ensure we've closed all <> pairs

jczhong84 · 2022-12-14T21:23:36Z

querybook/webapp/lib/utils/complex-types.ts

+    }
+ */
+export function parseType(key: string, type: string): ComplexType {
+    const regex = /(struct|array|map|uniontype)<(.*)>/i;


does it need to handle spaces? like struct <...>, struct<year : int, day : int>?

also should it match the start and end /^ ... $/?

I left spaces out intentionally as I haven't seen that in the wild. If needed it probably wouldn't be too hard, but didn't want to add unnecessary complexity.

Added start/end to the regex.

jczhong84 · 2022-12-14T21:40:12Z

querybook/webapp/lib/utils/complex-types.ts

+        if (char === '<') {
+            prettyString += '<\n';
+            depth += 1;
+            prettyString += '  '.repeat(depth);


could you use a variable for the 2 spaces ' '?

querybook/webapp/components/DataTableViewColumn/DataTableColumnCard.tsx

jczhong84 · 2022-12-14T22:07:54Z

querybook/webapp/components/DataTableViewColumn/DataTableColumnCard.tsx

+                    {complexType.type}
+                </StyledText>
+                <AccentText weight="extra">{complexType.key}</AccentText>


instead of displaying it as type key, I'd prefer to have it same as the ColumnRow, which is key type, which I think is more reasonable.

czgu · 2022-12-05T18:51:33Z

querybook/webapp/components/DataTableViewColumn/DataTableColumnCard.tsx

@@ -24,6 +26,7 @@ export const DataTableColumnCard: React.FunctionComponent<IProps> = ({
    updateDataColumnDescription,
 }) => {
    const [expanded, setExpanded] = React.useState(false);
+    const parsedType = parseType('', column.type);


czgu · 2022-12-05T19:02:40Z

querybook/webapp/components/DataTableViewColumn/DataTableColumnCard.tsx

@@ -71,3 +81,57 @@ export const DataTableColumnCard: React.FunctionComponent<IProps> = ({
        </div>
    );
 };
+
+interface IDataTableColumnCardNestedTypeProps {


can you separate this into another file?

czgu · 2023-01-06T21:03:57Z

querybook/webapp/components/DataTableViewColumn/DataTableColumnCardNestedType.tsx

+    IDataTableColumnCardNestedTypeProps
+> = ({ complexType }) => {
+    const hasChildren = complexType.children?.length > 0;
+    const [expanded, setExpanded] = React.useState(false);


you can use useToggleState then you don't need () => setExpanded(!expanded)

czgu · 2023-01-07T00:34:46Z

querybook/webapp/components/DataTableViewColumn/DataTableColumnCardNestedType.tsx

+    const hasChildren = complexType.children?.length > 0;
+    const [expanded, setExpanded] = React.useState(false);
+
+    const rowProps = {


const rowProps: React.HTMLAttributes = {}

czgu · 2023-01-07T00:36:45Z

querybook/webapp/components/DataTableViewColumn/DataTableColumnCardNestedType.tsx

+            </div>
+            {hasChildren &&
+                expanded &&
+                complexType.children?.map((child) => (


.map is sufficient

czgu · 2023-01-07T00:38:34Z

querybook/webapp/__tests__/lib/utils/complex-types.test.ts

+            'struct<ids:array<string>,data:uniontype<int,float,string>>'
+        )
+    ).toEqual(`struct<
+  ids:array<


should we add a space after : to make it easier to read?

czgu · 2023-01-07T00:49:01Z

querybook/webapp/components/DataTableViewMini/TablePanelView.tsx

@@ -53,6 +56,7 @@ export const TablePanelView: React.FunctionComponent<ITablePanelViewProps> = ({
                        onClick={onColumnRowClick.bind(null, col.id)}
                        name={col.name}
                        type={col.type}
+                        typeChildren={parseType('', col.type).children}


memo this?

actually, the parsing can be down in ColumnRow since col.type is passed as a prop

ColumnRow takes typeChildren because it only parses once, then recursively output nested rows. I've moved the parseType call down and inside useMemo, where it returns either the passed typeChildren or calculates it.

baumandm · 2023-01-10T00:24:36Z

Rebased to resolve conflicts / squashed the commits.

jczhong84 reviewed Dec 14, 2022

View reviewed changes

baumandm force-pushed the external/struct-types-v2 branch 2 times, most recently from 645b79f to a030692 Compare January 5, 2023 15:59

jczhong84 previously approved these changes Jan 6, 2023

View reviewed changes

czgu reviewed Jan 7, 2023

View reviewed changes

baumandm dismissed jczhong84’s stale review via e5e26b5 January 9, 2023 15:08

baumandm mentioned this pull request Jan 9, 2023

fix: Extend DataTableColumn type column length #1121

Merged

czgu previously approved these changes Jan 9, 2023

View reviewed changes

feat: Visualize complex Hive column types

963482e

baumandm dismissed czgu’s stale review via 963482e January 10, 2023 00:24

baumandm force-pushed the external/struct-types-v2 branch from e5e26b5 to 963482e Compare January 10, 2023 00:24

czgu approved these changes Jan 10, 2023

View reviewed changes

czgu merged commit 05de0ca into pinterest:master Jan 10, 2023

rohan-sh1 pushed a commit to CAI-TECHNOLOGIES/cai-ext-db-explorer that referenced this pull request Feb 28, 2023

feat: Visualize complex Hive column types (pinterest#1091)

eb9e7e2

baumandm deleted the external/struct-types-v2 branch September 21, 2023 16:55

aidenprice pushed a commit to arrowtail-precision/querybook that referenced this pull request Jan 3, 2024

feat: Visualize complex Hive column types (pinterest#1091)

9f10a39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Visualize complex Hive column types v2 #1091

feat: Visualize complex Hive column types v2 #1091

baumandm commented Dec 1, 2022

baumandm commented Dec 2, 2022

jczhong84 left a comment

jczhong84 Dec 14, 2022

baumandm Jan 5, 2023

jczhong84 Dec 14, 2022

baumandm Jan 5, 2023

baumandm Jan 5, 2023

jczhong84 Dec 14, 2022

jczhong84 Dec 14, 2022

czgu Dec 5, 2022

czgu Dec 5, 2022

czgu Jan 6, 2023

czgu Jan 7, 2023

czgu Jan 7, 2023

czgu Jan 7, 2023

czgu Jan 7, 2023

baumandm Jan 9, 2023

baumandm commented Jan 10, 2023

feat: Visualize complex Hive column types v2 #1091

feat: Visualize complex Hive column types v2 #1091

Conversation

baumandm commented Dec 1, 2022

baumandm commented Dec 2, 2022

jczhong84 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baumandm commented Jan 10, 2023