feat: views support #512
feat: views support #512
Conversation
depends on questdb/questdb#5720 disclosure: most of the code was generated by Claude Code
There was a problem hiding this comment.
Pull request overview
This PR adds support for views in the QuestDB web console schema explorer. The implementation introduces a new table_type field to differentiate between regular tables ('T'), materialized views ('M'), and views ('V'), providing backward compatibility with older servers that don't return this field.
Key Changes
- Added
TableTypeandViewtypes, plus new API methodsshowViewDDL()andshowViews() - Views are now categorized using the
table_typefield from the tables API instead of relying on separate state to distinguish materialized views from regular tables - Views display with a dedicated eye icon and exclude storage details (which are only relevant for physical tables)
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/questdb/types.ts | Added TableType and View type definitions, added optional table_type field to Table type for backward compatibility |
| src/utils/questdb/client.ts | Added showViewDDL() and showViews() API methods |
| src/scenes/Schema/table-icon.tsx | Added ViewIcon component and rendering logic with tooltip |
| src/scenes/Schema/localStorageUtils.ts | Added VIEWS_GROUP_KEY constant for localStorage |
| src/scenes/Schema/index.tsx | Integrated views fetching, DDL export support, and categorization logic based on table_type |
| src/scenes/Schema/VirtualTables/utils.ts | Updated createTableNode() to handle views, conditionally hide storage details for views |
| src/scenes/Schema/VirtualTables/index.tsx | Added views tree section, filtering logic, and context menu support |
| src/scenes/Schema/Row/index.tsx | Updated TreeNodeKind type and rendering logic to include "view" |
Comments suppressed due to low confidence (1)
src/scenes/Schema/VirtualTables/utils.ts:167
- Views are being passed table properties (partitionBy, walEnabled, designatedTimestamp) from the Table object, but regular views typically don't have these physical storage properties since they're virtual. These properties should either be undefined/null for views or the Table type returned by the server for views should handle this. Verify that the backend API returns appropriate values for these fields when table_type is 'V'.
partitionBy: table.partitionBy,
walEnabled: table.walEnabled,
designatedTimestamp: table.designatedTimestamp,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <SuspensionDialog | ||
| walTableData={item.walTableData} | ||
| kind={item.kind} | ||
| kind={item.kind as "table" | "matview"} |
There was a problem hiding this comment.
Type assertion might be unsafe here. The item.kind could theoretically be "view", but the SuspensionDialog is being passed item.kind as "table" | "matview" which excludes "view". While the code does check canSuspend before rendering the dialog (which ensures kind !== "view"), TypeScript doesn't recognize this relationship. Consider refactoring to make the type narrowing more explicit, or ensure SuspensionDialog can handle all valid kinds.
| placement="bottom" | ||
| > | ||
| <Tooltip> | ||
| View - a stored query that can be referenced like a table. |
There was a problem hiding this comment.
The tooltip text could be more consistent with the other tooltips in the file. Consider providing more details about what makes it different from materialized views, or keep it brief but informative like the materialized view tooltip.
| View - a stored query that can be referenced like a table. | |
| View – a stored SELECT query that returns results on demand and does not store data like a materialized view. |
| if (response?.type === QuestDB.Type.DQL && response.data?.[0]?.ddl) { | ||
| return response.data[0].ddl | ||
| } | ||
| tablesWithError.push(table) |
There was a problem hiding this comment.
The ddls array may contain undefined values when a response doesn't contain valid DDL data (line 251 is reached). This would cause issues when joining the array on line 258. Consider filtering out undefined values or explicitly returning undefined from the map function and filtering them out.
| if (shownIfFilteredSuspendedOnly && shownIfFilteredWithQuery) { | ||
| acc[table.matView ? 1 : 0].push({ | ||
| // Use table_type to categorize: 'T' = table, 'M' = matview, 'V' = view | ||
| // Default to 'T' (table) for backward compatibility with older servers | ||
| const tableType = table.table_type ?? "T" | ||
| const categoryIndex = tableType === "M" ? 1 : tableType === "V" ? 2 : 0 |
There was a problem hiding this comment.
Views are being filtered based on suspension status when filterSuspendedOnly is true. However, regular views typically don't have WAL support since they're virtual and cannot be suspended. This logic may incorrectly filter out views when the "Show suspended only" filter is active. Consider excluding views (table_type === 'V') from the suspension filter logic.
|
|
||
| async showViewDDL(viewName: string): Promise<QueryResult<{ ddl: string }>> { | ||
| return await this.query<{ ddl: string }>( | ||
| `SHOW CREATE VIEW '${viewName}';`, |
There was a problem hiding this comment.
SQL injection vulnerability: The viewName parameter is directly interpolated into the SQL query without any sanitization or parameterization. An attacker could pass a malicious viewName containing SQL commands. Consider using parameterized queries or properly escaping the input.
| void fetchMaterializedViews() | ||
| void fetchViews() |
There was a problem hiding this comment.
The materializedViews state variable is still being fetched and maintained, but it's no longer used in the allSelectableTables memo dependency array (line 353). However, it's still passed down to VirtualTables and used there to populate matViewData. This is intentional since materialized views are now identified via table_type === "M", but the separate materializedViews state is still needed to provide additional metadata like base_table_name and invalidation_reason. Consider adding a comment explaining why both the table_type field and the materializedViews state are needed.
| if (item.id === TABLES_GROUP_KEY || item.id === MATVIEWS_GROUP_KEY || item.id === VIEWS_GROUP_KEY) { | ||
| const isTable = item.id === TABLES_GROUP_KEY | ||
| const isMatView = item.id === MATVIEWS_GROUP_KEY | ||
| const isView = item.id === VIEWS_GROUP_KEY |
There was a problem hiding this comment.
Unused variable isView.
| const isView = item.id === VIEWS_GROUP_KEY |
depends on questdb/questdb#5720
disclosure: most of the code was generated by Claude Code