-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(web-console): add materialized views support #405
Conversation
bd0e64c
to
b50f1c4
Compare
835c46a
to
7136d48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congrats on first PR 😀 - I appreciate the approach & fearlessness.
A couple broad questions, a some comments, and a few visual/functional things.
🫡 Solid push.
It is not clear how to "use" the materialized view.
We can see them and copy the query. But there isn't any interactivity beyond that.
What is the goal of the current view?
Can we "help" more?
-
Link out to docs somewhere if there's no view? ie. better zero-state?
-
Instead of a copy, perhaps create a tab & launch the query on click?
-
If "materialized view" tab is active, execute query when appropriate?
Maybe beyond the scope of our first increment, and we might not have answers yet.
That's cool. 😃 But let's figure out where we're going.
"Invalid" states are missing.
There are conditions which can invalidate a mat view:
- Schema changes to the base table
- With deduplication, views must use same grouping keys as the base table's
UPSERT KEYS
- More
During these states, the view needs a REFRESH
. Do we guide through these cases?
More fatally, if the base table is dropped, there's no feedback.
For this, base table row is a potential space for warning and repair guidance:

The Table & Materialized view sections match base BG colour.

It seems off to me. Can we delineate them in a stylish way?
Maybe so tables & mat view look as "children" within data sources section?
Symmetry of the toggleable data sources button broke with filter change:

Before, it balanced with table view contents:

Co-authored-by: goodroot <9484709+goodroot@users.noreply.github.com>
Co-authored-by: goodroot <9484709+goodroot@users.noreply.github.com>
Co-authored-by: goodroot <9484709+goodroot@users.noreply.github.com>
9cc94d1
to
6d17ca8
Compare
@goodroot please take a look at this one: |
946d87f
to
18e8001
Compare
Hey @goodroot, I updated the PR with the design we agreed on the discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid work - the tree view feels fast and the ux is clean.
In another iteration, it'd be worthwhile to make the mat view section useful.
Right now, it's informational.
But "open query in tab" or something would go a long way.
I will have to step away for some time.
So, LGTM from here. 👍
GroupedVirtuoso
fromreact-virtuoso
. Reduces render time in case we have 1000s of tables / mat. views.show create materialized view...
instead ofshow create table
.react-contextmenu
with@radix-ui/react-context-menu
due to bugs.