fix(data-warehouse): restore sync method column for non-SQL sources#60908
Conversation
👥 Auto-assigned reviewersSkipped a review request for |
|
Reviews (1): Last reviewed commit: "fix(data-warehouse): restore sync method..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Restores the Sync method (and related sync configuration) columns in the data warehouse “Select tables to import” step for non-SQL (REST/webhook) sources by separating column-selection visibility from sync-configuration visibility.
Changes:
- Splits the previous
shouldShowSyncColumnsgate into:shouldShowSyncColumns = !isDirectQueryMode(sync field / primary key / sync method)shouldShowColumnSelection = !isDirectQueryMode && !!selectedConnector?.supportsColumnSelection(columns picker)
- Ensures the Columns column remains restricted to SQL sources that support column selection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const shouldShowSyncColumns = !isDirectQueryMode | ||
| const shouldShowColumnSelection = !isDirectQueryMode && !!selectedConnector?.supportsColumnSelection | ||
|
|
|
Size Change: 0 B Total Size: 80.9 MB ℹ️ View Unchanged
|
Problem
In the new source wizard's "Select tables to import" step, the Sync method column disappeared for non-SQL sources (Stripe, Hubspot, and any REST-based source). Users could no longer choose the sync method (full refresh vs incremental) per table during source setup.
This was a regression from #60155, which added per-table column selection for SQL sources (MySQL, MSSQL, BigQuery, Snowflake, Redshift, Postgres).
Changes
The column-selection PR tightened the existing
shouldShowSyncColumnsgate to!isDirectQueryMode && !!selectedConnector?.supportsColumnSelection. That variable also controls the Sync field, Primary key, and Sync method columns — so gating it behindsupportsColumnSelection(which isfalsefor all non-SQL sources) hid those columns everywhere except SQL sources.This splits the gate into two:
shouldShowSyncColumns = !isDirectQueryMode— restores the original behavior for Sync field, Primary key, and Sync method (shown for all non-direct-query sources).shouldShowColumnSelection = !isDirectQueryMode && !!selectedConnector?.supportsColumnSelection— keeps the new Columns column correctly scoped to SQL sources, which was the intent of feat(data-warehouse): column selection on MySQL, MSSQL, BigQuery, Snowflake, Redshift #60155.How did you test this code?
I'm an agent (Claude Code). I traced the regression to the gating change in
SchemaForm.tsx, confirmed viagit showthat #60155 was the source, and verifiedsupports_column_selectiondefaults toFalsein the source base config (only SQL sources set itTrue). I did not run the wizard manually — a quick manual check that the Sync method picker renders and works for a Stripe source would be worth doing before merge. No automated tests cover this column-gating logic.🤖 Agent context
Authored with Claude Code. The user reported the missing sync method picker for Stripe; I located the table-selection step (
SchemaForm), bisected the cause to the over-broadshouldShowSyncColumnsgate introduced in #60155, and decoupled the column-selection gate from the sync-method/sync-field/primary-key gate rather than reverting the column-selection feature.