Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 145 additions & 52 deletions ui/app/mirrors/create/cdc/columnbox.tsx
Original file line number Diff line number Diff line change
@@ -1,88 +1,181 @@
'use client';
import { TableMapRow } from '@/app/dto/MirrorsDTO';
import { ColumnSetting } from '@/grpc_generated/flow';
import { ColumnsItem } from '@/grpc_generated/route';
import { Checkbox } from '@/lib/Checkbox';
import { Label } from '@/lib/Label';
import { RowWithCheckbox } from '@/lib/Layout';
import { Dispatch, SetStateAction } from 'react';
import { Dispatch, Fragment, SetStateAction, useMemo } from 'react';
import InfoPopover from '@/components/InfoPopover';
Comment on lines 5 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { Checkbox } from '@/lib/Checkbox';
import { Label } from '@/lib/Label';
import { RowWithCheckbox } from '@/lib/Layout';
import { Dispatch, SetStateAction } from 'react';
import { Dispatch, Fragment, SetStateAction, useMemo } from 'react';
import InfoPopover from '@/components/InfoPopover';
import { appTheme } from '@/lib/AppTheme/appTheme';
import { Checkbox } from '@/lib/Checkbox';
import { Label } from '@/lib/Label';
import { RowWithCheckbox } from '@/lib/Layout';
import { Dispatch, Fragment, SetStateAction, useMemo } from 'react';


interface ColumnProps {
columns: ColumnsItem[];
tableRow: TableMapRow;
rows: TableMapRow[];
setRows: Dispatch<SetStateAction<TableMapRow[]>>;
disabled?: boolean;
showOrdering: boolean;
showNullable: boolean;
}
export default function ColumnBox({
columns,
tableRow,
rows,
setRows,
disabled,
showNullable,
}: ColumnProps) {
const handleColumnExclusion = (column: string, include: boolean) => {
const source = tableRow.source;
const currRows = [...rows];
const rowIndex = currRows.findIndex((row) => row.source === source);
// Helper to update a specific row
const updateRow = (updater: (row: TableMapRow) => TableMapRow) => {
const rowIndex = rows.findIndex((row) => row.source === tableRow.source);
if (rowIndex !== -1) {
const sourceRow = currRows[rowIndex],
newExclude = new Set(sourceRow.exclude);
const updatedRows = [...rows];
updatedRows[rowIndex] = updater(updatedRows[rowIndex]);
setRows(updatedRows);
}
};

const handleColumnExclusion = (column: string, include: boolean) => {
updateRow((row) => {
const newExclude = new Set(row.exclude);
if (include) {
newExclude.delete(column);
} else {
newExclude.add(column);
}
currRows[rowIndex] = {
...sourceRow,
exclude: newExclude,
};
setRows(currRows);
}
return { ...row, exclude: newExclude };
});
};

return columns.map((column) => {
const partOfOrderingKey = rows
.find((row) => row.source == tableRow.source)
?.columns.some(
(col) =>
col.sourceName === column.name &&
(col.ordering > 0 || col.partitioning > 0)
const handleNullableEnabledChange = (columnName: string, enabled: boolean) => {
updateRow((row) => {
const existingColumn = row.columns.find(
(col) => col.sourceName === columnName
);
return (
<RowWithCheckbox
key={column.name}
label={
<Label
as='label'
style={{
fontSize: 13,
display: 'flex',
alignItems: 'center',
}}
>
{column.name}
<p

const updatedColumns: ColumnSetting[] = existingColumn
? // Update existing ColumnSetting
row.columns.map((col) =>
col.sourceName === columnName
? { ...col, nullableEnabled: enabled }
: col
)
: // Create new ColumnSetting
[
...row.columns,
{
sourceName: columnName,
destinationName: '',
destinationType: '',
ordering: 0,
partitioning: 0,
nullableEnabled: enabled,
},
];

return { ...row, columns: updatedColumns };
});
};

const nullableEnabledMap = useMemo(() => {
const map = new Map<string, boolean>();
tableRow.columns.forEach((col) => {
map.set(col.sourceName, col.nullableEnabled ?? false);
});
return map;
}, [tableRow.columns]);

const getNullableEnabled = (columnName: string): boolean => {
return nullableEnabledMap.get(columnName) ?? false;
};

return (
<div
style={{
display: 'grid',
gridTemplateColumns: showNullable
? 'minmax(0, 2fr) minmax(0, 1fr) auto'
: 'minmax(0, 2fr) minmax(0, 1fr)',
alignItems: 'center',
columnGap: '2.5rem',
width: '80%',
}}
>
<div style={{ fontSize: 12, fontWeight: 500, textAlign: 'left' }}>Name</div>
<div style={{ fontSize: 12, fontWeight: 500 }}>Type</div>
{showNullable && (
<div style={{ fontSize: 12, fontWeight: 500, display: 'flex', alignItems: 'center', gap: '0.25rem', justifyContent: 'flex-end' }}>
Nullable
<InfoPopover
tips="Enabling nullable columns wraps types in Nullable() which may impact query performance and storage in Clickhouse."
/>
</div>
)}
Comment on lines +92 to +112
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div
style={{
display: 'grid',
gridTemplateColumns: showNullable
? 'minmax(0, 2fr) minmax(0, 1fr) auto'
: 'minmax(0, 2fr) minmax(0, 1fr)',
alignItems: 'center',
columnGap: '2.5rem',
width: '80%',
}}
>
<div style={{ fontSize: 12, fontWeight: 500, textAlign: 'left' }}>Name</div>
<div style={{ fontSize: 12, fontWeight: 500 }}>Type</div>
{showNullable && (
<div style={{ fontSize: 12, fontWeight: 500, display: 'flex', alignItems: 'center', gap: '0.25rem', justifyContent: 'flex-end' }}>
Nullable
<InfoPopover
tips="Enabling nullable columns wraps types in Nullable() which may impact query performance and storage in Clickhouse."
/>
</div>
)}
<div
style={{
display: 'grid',
gridTemplateColumns: showNullable
? 'auto auto auto'
: 'auto auto',
alignItems: 'center',
columnGap: '3rem',
width: 'fit-content',
}}
>
<div
style={{
fontSize: 12,
fontWeight: 500,
textAlign: 'left',
paddingLeft: `calc(${appTheme.spacing.xxSmall} + ${appTheme.spacing.medium})`,
}}
>
Name
</div>
<div style={{ fontSize: 12, fontWeight: 500 }}>Type</div>
{showNullable && (
<div style={{ fontSize: 12, fontWeight: 500 }}>Nullable</div>
)}


{columns.map((column) => {
const partOfOrderingKey = rows
.find((row) => row.source == tableRow.source)
?.columns.some(
(col) =>
col.sourceName === column.name &&
(col.ordering > 0 || col.partitioning > 0)
);

const isIncluded = !tableRow.exclude.has(column.name);
const nullableEnabled = getNullableEnabled(column.name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is O(N^2) in the number of columns, could that be improved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can use a map for O(1) lookups.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still O(N^2) :)


return (
<Fragment key={column.name}>
<RowWithCheckbox
label={
<Label
as="label"
style={{
fontSize: 13,
display: 'flex',
alignItems: 'center',
}}
>
{column.name}
</Label>
}
action={
<Checkbox
style={{ cursor: 'pointer' }}
disabled={column.isKey || disabled || partOfOrderingKey}
checked={isIncluded}
onCheckedChange={(state: boolean) =>
handleColumnExclusion(column.name, state)
}
/>
}
/>

<div
style={{
marginLeft: '0.5rem',
fontSize: 13,
color: 'gray',
whiteSpace: 'nowrap',
}}
>
{column.type}
</p>
</Label>
}
action={
<Checkbox
style={{ cursor: 'pointer' }}
disabled={column.isKey || disabled || partOfOrderingKey}
checked={!tableRow.exclude.has(column.name)}
onCheckedChange={(state: boolean) =>
handleColumnExclusion(column.name, state)
}
/>
}
/>
);
});
</div>

{showNullable && (
<Checkbox
style={{
cursor: isIncluded && !disabled ? 'pointer' : 'default',
justifySelf: 'flex-end',
}}
disabled={disabled || !isIncluded}
checked={isIncluded ? nullableEnabled : false}
onCheckedChange={(state: boolean) =>
handleNullableEnabledChange(column.name, state)
}
/>
)}
</Fragment>
);
})}
</div>
);
}
4 changes: 2 additions & 2 deletions ui/app/mirrors/create/cdc/customColumnType.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ export default function CustomColumnType({
prev.map((row) => {
if (row.source !== tableRow.source) return row;

const columnExistsInRow = row.columns.some(
const existingColumn = row.columns.find(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Contributor Author

@Br0wnHammer Br0wnHammer Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Find helps in getting the column object. Instead of a boolean. Which we can use for future proofing. If needed I can revert to as it was before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now it is just putting more work on the runtime while we only need a boolean. Let's leave this change for when the future actually comes.

(col) => col.sourceName === selectedColumnName
);
if (!columnExistsInRow) {
if (!existingColumn) {
return {
...row,
columns: [
Expand Down
3 changes: 2 additions & 1 deletion ui/app/mirrors/create/cdc/schemabox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ export default function SchemaBox({
row.partitionByExpr = existingRow.partitionByExpr;
row.exclude = new Set(existingRow.exclude ?? []);
row.destination = existingRow.destinationTableIdentifier;
row.columns = existingRow.columns ?? [];
addTableColumns(row.source);
}
}
Expand Down Expand Up @@ -529,7 +530,7 @@ export default function SchemaBox({
rows={rows}
setRows={setRows}
disabled={row.editingDisabled}
showOrdering={
showNullable={
peerType?.toString() ===
DBType[DBType.CLICKHOUSE].toString()
}
Expand Down
6 changes: 5 additions & 1 deletion ui/app/mirrors/create/cdc/sortingkey.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ function UpdatedRows(
const rowIndex = rows.findIndex((row) => row.source === tableRow.source);
if (rowIndex !== -1) {
const newRows = [...rows];
// Check if there's an existing ColumnSetting to preserve nullableEnabled
const existingSetting = rows[rowIndex].columns.find(
(setting) => setting.sourceName === colName
);
const columns = rows[rowIndex].columns.map(update);
if (!columns.find((setting) => setting.sourceName === colName)) {
columns.push(
Expand All @@ -58,7 +62,7 @@ function UpdatedRows(
destinationType: '',
ordering: 0,
partitioning: 0,
nullableEnabled: false,
nullableEnabled: existingSetting?.nullableEnabled ?? false,
})
);
}
Expand Down
Loading