feat(data-exploration): improve data table crashes #13806
Conversation
…o hogql-small-refactor
| if (value === loadingColumn) { | ||
| return <Spinner /> | ||
| } else if (value === errorColumn) { | ||
| return <Tag color="red">Error</Tag> |
There was a problem hiding this comment.
You're giving me extra work 😄 Why not LemonTag?
There was a problem hiding this comment.
I didn't find it 🤪
| (highlightedRows) => | ||
| (row: any[]): boolean => | ||
| row | ||
| ? highlightedRows.has('__originalResultRow' in row ? (row as any).__originalResultRow : row) |
There was a problem hiding this comment.
Can't we add __originalResultRow to the type?
There was a problem hiding this comment.
Makes sense. Converted all the results to DataTableRow-s and it was as if all the windows were opened and light could finally shine through.
| } | ||
| } | ||
| } | ||
| return response && 'results' in response ? (response as any).results ?? null : null | ||
| }, | ||
| ], | ||
| isRowHighlighted: [ |
There was a problem hiding this comment.
Hm I can't quite grasp the purpose of this highlighting from the code. Can you add a one-line comment explaining what rows we want highlighted?
There was a problem hiding this comment.
The row data structure refactor removed the need for this proxying. yeet!
There was a problem hiding this comment.
Noticed a slightly weird thing UX-wise:
If I first add an invalid expression column (e.g. !!!) and then a valid one (e.g. properties.$geoip_country_code), both will show up as "Error"ing. And even when I delete the invalid one, the valid one will still be all "Error"s. If we could somehow determine which column is the offending one, this would be more intuitive.
Can we do this easily, considering we already catch problems like the column not existing in property_access_to_clickhouse, or would that be overkill for now?
|
I went for a slightly simpler solution for now: just remove the results when we have an error. This also makes it easier to actually show errors when they occur.
|
Problem
The data exploration table would crash in weird ways when adding columns that didn't make sense. You'd have to go and edit the query to recover from this state:
Changes
Now it either shows a full page "Error", or then uses a antd Tag to show the error in the table.
The error component is stolen from insights. I also added a "no results" message, again from insights:
Out of scope
There's a separate PR out that exposes the actual errors themselves to the users:
#13703
This PR wants to do the minimum work in order to let the users recover from a common scenario of "I added faulty hogql, how do I recover?".
How did you test this code?
Made it crash.