feat: Add RawSqlChartConfig types for SQL-based Table#1846
feat: Add RawSqlChartConfig types for SQL-based Table#1846
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 66f5240 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Claude finished @pulpdrew's task in 45s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
Overall: This is a well-structured refactor adding A few items worth addressing:
✅ No critical blocking issues — the type architecture is sound and the narrowing guards are consistently applied across ~65 files. The security consideration around |
E2E Test Results✅ All tests passed • 84 passed • 3 skipped • 968s
Tests ran across 4 shards in parallel. |
d4507d5 to
b49707e
Compare
|
Claude finished @pulpdrew's task in 1m 5s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
Overall the discriminated union approach is clean and well-executed. A few items worth addressing:
|
|
Claude finished @pulpdrew's task in 39s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
✅ The discriminated union approach is clean and the A few remaining items:
|
3066822 to
8f9f741
Compare
|
Claude finished @pulpdrew's task in 27s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
✅ The discriminated union approach is well-structured and consistently applied across the codebase. No critical blocking issues found. One remaining item from prior reviews:
|
| return result; | ||
| }; | ||
|
|
||
| const convertChartConfigToExternalChartSeries = ( |
There was a problem hiding this comment.
These functions were unused, easier to delete them now than update the types.
8f9f741 to
fa4e6a2
Compare
|
Claude finished @pulpdrew's task in 1m 0s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
✅ The discriminated union approach is clean and consistently applied. Previous review concerns about inconsistent guards in One remaining actionable item:
ℹ️ |
fa4e6a2 to
502a599
Compare
|
Claude finished @pulpdrew's task in 41s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
✅ The discriminated union approach is clean and well-executed. Type guards in Two remaining items:
|
502a599 to
29250e8
Compare
|
Claude finished @pulpdrew's task in 27s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table✅ The discriminated union approach is clean and well-executed. Previous concerns about inconsistent guards between converters are resolved — both One remaining item:
|
|
Claude finished @pulpdrew's task in 31s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table✅ The discriminated union approach is clean and well-structured. Type guards in Two remaining items:
|
c1e88c9 to
d5e5a82
Compare
|
Claude finished @pulpdrew's task in 32s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table✅ The discriminated union approach is clean and well-structured. Type guards in Two remaining items carried over from prior reviews:
|
|
Claude finished @pulpdrew's task in 43s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
✅ The discriminated union approach is well-structured and consistently applied. Previous concerns about inconsistent guards between One remaining actionable item:
|
2e08b42 to
fb5531a
Compare
|
Claude finished @pulpdrew's task in 40s —— View job PR Review: feat: Add RawSqlChartConfig types for SQL-based Table
✅ The discriminated union approach is well-structured and the type guards in One remaining actionable item that has persisted across all prior reviews:
|
| return []; | ||
| } | ||
|
|
||
| if (isRawSqlSavedChartConfig(tile.config)) { |
There was a problem hiding this comment.
Do we plan to support this eventually? If so, should we add a comment with a linked issue?
There was a problem hiding this comment.
There are no immediate plans to support this.
| onChange={(value: string) => { | ||
| onChange(value == 'sql' ? 'sql' : 'builder'); | ||
| }} |
There was a problem hiding this comment.
I think this can be simplified
| onChange={(value: string) => { | |
| onChange(value == 'sql' ? 'sql' : 'builder'); | |
| }} | |
| onChange={onChange} |
|
|
||
| import resizeStyles from '@/../styles/ResizablePanel.module.scss'; | ||
|
|
||
| export default function RawSqlChartEditor({ |
There was a problem hiding this comment.
If it doesn't exist, we should create a follow up issue that makes it really easy to use a daterange from a date picker. Like a macro replacement in your SQL query or something. It's much more natural to use a date picker to select times than dealing with timestamps and typing out WHERE Timestamp > toDateTime(0023187932719) AND Timestamp < toDateTime(00348391784931)
There was a problem hiding this comment.
Actually I do see the datetime there, but it's in a weird spot. I feel it should be inline with the connection selector.
There was a problem hiding this comment.
Also some UX indicator that the date range is being applied would probably be helpful. Even a "disable date range" button that changes an icon, similar to accelerated symbols for materialization
There was a problem hiding this comment.
100% agree - already working on it in this followup PR
| /> | ||
| </Group> | ||
| <Box style={{ position: 'relative' }}> | ||
| <SQLEditorControlled |
There was a problem hiding this comment.
This does not currently line wrap, but should
(query for reference: select toStartOfMinute(Timestamp) as minuteslice, SeverityText, count() as severity_count from otel_logs WHERE Timestamp > toDateTime(1772722398) - INTERVAL 15 MINUTE AND Timestamp < toDateTime(1772722398) group by minuteslice, SeverityText ORDER BY minuteslice DESC)
There was a problem hiding this comment.
Added line wrapping
|
|
||
| import resizeStyles from '@/../styles/ResizablePanel.module.scss'; | ||
|
|
||
| export default function RawSqlChartEditor({ |
There was a problem hiding this comment.
I can run truncate otel_logs and delete all data in the table. We should have some parsing at least to ensure we are only running SELECT queries
| **/ | ||
| const SharedChartDisplaySettingsSchema = z.object({ | ||
| displayType: z.nativeEnum(DisplayType).optional(), | ||
| numberFormat: NumberFormatSchema.optional(), |
There was a problem hiding this comment.
I agree, though this also applies to non-raw-sql based charts. Created an issue 👍

Summary
This PR is the first step towards raw SQL-driven charts.
BuilderChartConfig(which is unchanged from the currentChartConfigtypesandRawSqlChartConfig` types which represent sql-driven charts.The changes in most of the files in this PR are either type updates or the addition of type guards to handle the new ChartConfig union type.
The DBEditTimeChartForm has been updated significantly to (a) add the Raw SQL option to the table chart editor and (b) handle conversion from internal form state (which can now include properties from either branch of the ChartConfig union) to valid SavedChartConfigs (which may only include properties from one branch).
Significant changes are in:
Future PRs will add templating to the Raw SQL driven charts for date range and granularity injection; support for other chart types driven by SQL; improved placeholder, validation, and error states; and improved support in the external API and import/export.
Screenshots or video
Screen.Recording.2026-03-04.at.4.45.58.PM.mov
How to test locally or on Vercel
The SQL-driven table can be tested in the preview environment or locally.
References