villebro commented on a change in pull request #16334:
URL: https://github.com/apache/superset/pull/16334#discussion_r692789752
##########
File path: superset-frontend/src/components/DatabaseSelector/index.tsx
##########
@@ -97,193 +89,189 @@ export default function DatabaseSelector({
schema,
sqlLabMode = false,
}: DatabaseSelectorProps) {
- const [currentDbId, setCurrentDbId] = useState(dbId);
- const [currentSchema, setCurrentSchema] = useState<string | undefined>(
- schema,
+ const [currentDb, setCurrentDb] = useState(
+ db
+ ? { label: `${db.backend}: ${db.database_name}`, value: db.id }
+ : undefined,
+ );
+ const [currentSchema, setCurrentSchema] = useState<SchemaValue | undefined>(
+ schema ? { label: schema, value: schema } : undefined,
);
- const [schemaLoading, setSchemaLoading] = useState(false);
- const [schemaOptions, setSchemaOptions] = useState([]);
+ const [refresh, setRefresh] = useState(0);
- function fetchSchemas(databaseId: number, forceRefresh = false) {
- const actualDbId = databaseId || dbId;
- if (actualDbId) {
- setSchemaLoading(true);
- const queryParams = rison.encode({
- force: Boolean(forceRefresh),
- });
- const endpoint =
`/api/v1/database/${actualDbId}/schemas/?q=${queryParams}`;
- return SupersetClient.get({ endpoint })
- .then(({ json }) => {
+ const loadSchemas = useMemo(
+ () => async (): Promise<{
+ data: SchemaValue[];
+ totalCount: number;
+ }> => {
+ if (currentDb) {
+ const queryParams = rison.encode({ force: refresh > 0 });
+ const endpoint =
`/api/v1/database/${currentDb.value}/schemas/?q=${queryParams}`;
+
+ // The endpoint currently does not support pagination
+ return SupersetClient.get({ endpoint }).then(({ json }) => {
Review comment:
Could potentially change this to a `TODO:` if you have to push commits
to the PR (would be nice to add pagination in a follow-up)
##########
File path: superset-frontend/src/components/TableSelector/index.tsx
##########
@@ -129,179 +175,173 @@ const TableSelector:
FunctionComponent<TableSelectorProps> = ({
tableName,
tableNameSticky = true,
}) => {
+ const [currentDbId, setCurrentDbId] = useState<number | undefined>(dbId);
const [currentSchema, setCurrentSchema] = useState<string | undefined>(
schema,
);
- const [currentTableName, setCurrentTableName] = useState<string | undefined>(
- tableName,
- );
- const [tableLoading, setTableLoading] = useState(false);
- const [tableOptions, setTableOptions] = useState([]);
-
- function fetchTables(
- databaseId?: number,
- schema?: string,
- forceRefresh = false,
- substr = 'undefined',
- ) {
- const dbSchema = schema || currentSchema;
- const actualDbId = databaseId || dbId;
- if (actualDbId && dbSchema) {
- const encodedSchema = encodeURIComponent(dbSchema);
- const encodedSubstr = encodeURIComponent(substr);
- setTableLoading(true);
- setTableOptions([]);
+ const [currentTable, setCurrentTable] = useState<TableOption | undefined>();
+ const [refresh, setRefresh] = useState(0);
+
+ const loadTable = useMemo(
+ () => async (dbId: number, schema: string, tableName: string) => {
const endpoint = encodeURI(
-
`/superset/tables/${actualDbId}/${encodedSchema}/${encodedSubstr}/${!!forceRefresh}/`,
+ `/superset/tables/${dbId}/${schema}/${encodeURIComponent(
+ tableName,
+ )}/true/true`,
);
- return SupersetClient.get({ endpoint })
- .then(({ json }) => {
- const options = json.options.map((o: any) => ({
- value: o.value,
- schema: o.schema,
- label: o.label,
- title: o.title,
- type: o.type,
- extra: o?.extra,
- }));
- setTableLoading(false);
- setTableOptions(options);
+ return SupersetClient.get({ endpoint }).then(({ json }) => {
+ const options = json.options as Table[];
+ if (options && options.length > 0) {
+ return options[0];
+ }
+ return null;
+ });
+ },
+ [],
+ );
+
+ const loadTables = useMemo(
+ () => async (search: string) => {
+ const dbSchema = schema || currentSchema;
+ if (currentDbId && dbSchema) {
+ const encodedSchema = encodeURIComponent(dbSchema);
+ const encodedSubstr = encodeURIComponent(search || 'undefined');
+ const endpoint = encodeURI(
+
`/superset/tables/${currentDbId}/${encodedSchema}/${encodedSubstr}/true/`,
Review comment:
This should also default to not force updating the metadata from the db
```suggestion
`/superset/tables/${currentDbId}/${encodedSchema}/${encodedSubstr}/false/`,
```
##########
File path: superset-frontend/src/components/TableSelector/index.tsx
##########
@@ -129,179 +175,173 @@ const TableSelector:
FunctionComponent<TableSelectorProps> = ({
tableName,
tableNameSticky = true,
}) => {
+ const [currentDbId, setCurrentDbId] = useState<number | undefined>(dbId);
const [currentSchema, setCurrentSchema] = useState<string | undefined>(
schema,
);
- const [currentTableName, setCurrentTableName] = useState<string | undefined>(
- tableName,
- );
- const [tableLoading, setTableLoading] = useState(false);
- const [tableOptions, setTableOptions] = useState([]);
-
- function fetchTables(
- databaseId?: number,
- schema?: string,
- forceRefresh = false,
- substr = 'undefined',
- ) {
- const dbSchema = schema || currentSchema;
- const actualDbId = databaseId || dbId;
- if (actualDbId && dbSchema) {
- const encodedSchema = encodeURIComponent(dbSchema);
- const encodedSubstr = encodeURIComponent(substr);
- setTableLoading(true);
- setTableOptions([]);
+ const [currentTable, setCurrentTable] = useState<TableOption | undefined>();
+ const [refresh, setRefresh] = useState(0);
+
+ const loadTable = useMemo(
+ () => async (dbId: number, schema: string, tableName: string) => {
const endpoint = encodeURI(
-
`/superset/tables/${actualDbId}/${encodedSchema}/${encodedSubstr}/${!!forceRefresh}/`,
+ `/superset/tables/${dbId}/${schema}/${encodeURIComponent(
+ tableName,
+ )}/true/true`,
);
Review comment:
This will force table metadata to be refetched from the db as opposed to
the cache. We should probably retain the `forceRefresh` variable and pass it
along here, like
```typescript
const loadTable = useMemo(
() => async (dbId: number, schema: string, tableName: string,
forceRefresh: boolean) => {
const endpoint = encodeURI(
`/superset/tables/${dbId}/${schema}/${encodeURIComponent(
tableName,
)}/${forceRefresh}/true`,
);```
##########
File path: superset-frontend/src/components/TableSelector/index.tsx
##########
@@ -129,179 +175,173 @@ const TableSelector:
FunctionComponent<TableSelectorProps> = ({
tableName,
tableNameSticky = true,
}) => {
+ const [currentDbId, setCurrentDbId] = useState<number | undefined>(dbId);
const [currentSchema, setCurrentSchema] = useState<string | undefined>(
schema,
);
- const [currentTableName, setCurrentTableName] = useState<string | undefined>(
- tableName,
- );
- const [tableLoading, setTableLoading] = useState(false);
- const [tableOptions, setTableOptions] = useState([]);
-
- function fetchTables(
- databaseId?: number,
- schema?: string,
- forceRefresh = false,
- substr = 'undefined',
- ) {
- const dbSchema = schema || currentSchema;
- const actualDbId = databaseId || dbId;
- if (actualDbId && dbSchema) {
- const encodedSchema = encodeURIComponent(dbSchema);
- const encodedSubstr = encodeURIComponent(substr);
- setTableLoading(true);
- setTableOptions([]);
+ const [currentTable, setCurrentTable] = useState<TableOption | undefined>();
+ const [refresh, setRefresh] = useState(0);
+
+ const loadTable = useMemo(
+ () => async (dbId: number, schema: string, tableName: string) => {
const endpoint = encodeURI(
-
`/superset/tables/${actualDbId}/${encodedSchema}/${encodedSubstr}/${!!forceRefresh}/`,
+ `/superset/tables/${dbId}/${schema}/${encodeURIComponent(
+ tableName,
+ )}/true/true`,
);
- return SupersetClient.get({ endpoint })
- .then(({ json }) => {
- const options = json.options.map((o: any) => ({
- value: o.value,
- schema: o.schema,
- label: o.label,
- title: o.title,
- type: o.type,
- extra: o?.extra,
- }));
- setTableLoading(false);
- setTableOptions(options);
+ return SupersetClient.get({ endpoint }).then(({ json }) => {
+ const options = json.options as Table[];
+ if (options && options.length > 0) {
+ return options[0];
+ }
+ return null;
+ });
+ },
+ [],
+ );
+
+ const loadTables = useMemo(
+ () => async (search: string) => {
+ const dbSchema = schema || currentSchema;
+ if (currentDbId && dbSchema) {
+ const encodedSchema = encodeURIComponent(dbSchema);
+ const encodedSubstr = encodeURIComponent(search || 'undefined');
+ const endpoint = encodeURI(
+
`/superset/tables/${currentDbId}/${encodedSchema}/${encodedSubstr}/true/`,
+ );
+ return SupersetClient.get({ endpoint }).then(({ json }) => {
+ const options = json.options
+ .map((table: Table) => ({
+ value: table.value,
+ label: <TableOption table={table} />,
+ text: table.label,
+ }))
+ .sort((a: { text: string }, b: { text: string }) =>
+ a.text.localeCompare(b.text),
+ );
+
if (onTablesLoad) {
onTablesLoad(json.options);
}
- })
- .catch(() => {
- setTableLoading(false);
- setTableOptions([]);
- handleError(t('Error while fetching table list'));
+
+ return {
+ data: options,
+ totalCount: options.length,
+ };
});
- }
- setTableLoading(false);
- setTableOptions([]);
- return Promise.resolve();
- }
+ }
+ return { data: [], totalCount: 0 };
+ },
+ // We are using the refresh state to re-trigger the query
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ [currentDbId, currentSchema, onTablesLoad, schema, refresh],
+ );
useEffect(() => {
- if (dbId && schema) {
- fetchTables();
+ async function fetchTable() {
+ if (schema && tableName) {
+ const table = await loadTable(dbId, schema, tableName);
+ if (table) {
+ setCurrentTable({
+ label: <TableOption table={table} />,
+ text: table.label,
+ value: table.value,
+ });
+ }
+ }
}
- }, [dbId, schema]);
+ fetchTable();
+ }, [dbId, loadTable, schema, tableName]);
function onSelectionChange({
dbId,
schema,
- tableName,
+ table,
}: {
dbId: number;
schema?: string;
- tableName?: string;
+ table?: TableOption;
}) {
- setCurrentTableName(tableName);
+ setCurrentTable(table);
+ setCurrentDbId(dbId);
setCurrentSchema(schema);
if (onUpdate) {
- onUpdate({ dbId, schema, tableName });
+ onUpdate({ dbId, schema, tableName: table?.value });
}
}
- function getTableNamesBySubStr(substr = 'undefined') {
- if (!dbId || !substr) {
- const options: any[] = [];
- return Promise.resolve({ options });
- }
- const encodedSchema = encodeURIComponent(schema || '');
- const encodedSubstr = encodeURIComponent(substr);
- return SupersetClient.get({
- endpoint: encodeURI(
- `/superset/tables/${dbId}/${encodedSchema}/${encodedSubstr}`,
- ),
- }).then(({ json }) => {
- const options = json.options.map((o: any) => ({
- value: o.value,
- schema: o.schema,
- label: o.label,
- title: o.title,
- type: o.type,
- }));
- return { options };
- });
- }
-
- function changeTable(tableOpt: any) {
- if (!tableOpt) {
- setCurrentTableName('');
+ function changeTable(table: TableOption) {
+ if (!table) {
+ setCurrentTable(undefined);
return;
}
- const schemaName = tableOpt.schema;
- const tableOptTableName = tableOpt.value;
- if (tableNameSticky) {
+ const tableOptTableName = table.value;
+ if (currentDbId && tableNameSticky) {
onSelectionChange({
- dbId,
- schema: schemaName,
- tableName: tableOptTableName,
+ dbId: currentDbId,
+ schema: currentSchema,
+ table,
});
}
- if (onTableChange) {
- onTableChange(tableOptTableName, schemaName);
+ if (onTableChange && currentSchema) {
+ onTableChange(tableOptTableName, currentSchema);
}
}
- function changeSchema(schemaOpt: any, force = false) {
- const value = schemaOpt ? schemaOpt.value : null;
+ function onRefresh() {
Review comment:
I think we need to add a force param here that the force refresh calls.
Something like
```js
function onRefresh(forceRefresh: boolean) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
