-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Visualize SqlLab.Query model data in Explore 📈 #20281
Changes from 41 commits
6fc435d
632ce21
6f22618
236c489
24261d1
bd3b6a9
64f4181
7c07534
6fab57f
3a46152
613d78a
e076f13
31dfdd2
bbb91e0
8513cc5
cd54603
a173cfa
0295818
de20276
bd7db0d
e344197
9c5bc48
b6fbc33
6af2961
12546f5
ff59846
bdbf2d1
559e1a2
890be43
4b833be
4254b7e
4710b3d
017bde6
075c5d8
d73504c
74ae38c
97ee4eb
276bc28
a8ff273
044945b
8d8a868
7ace7a4
e436582
76c3505
3370439
d1866e8
18b6fa1
a7ded35
c65c225
232d6dc
aa2756e
ebdfcf0
a52acd9
3653e40
c0ab26b
655fddf
f752655
c478add
5e32c24
6707808
b28d79c
c619618
fa85488
a09d781
4850f9a
bb7acbb
d80576e
076e4fd
c5ca22d
a35038d
661bb9e
b476b70
56fcdc9
551ddc8
6e5443a
97ac731
664773a
a073313
f717e18
16ea687
3f6fa0a
88a51b4
a105ca6
9e09af2
8eb1709
744f031
d21f145
432aee7
66fc9a6
ff963db
e4448fe
a203bf8
15d530c
48f40a6
555d26b
cc7a2d0
eb99ad4
4fa62e9
afa34fd
c0d7c14
0666568
0e5767b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ export default class DatasourceKey { | |
this.id = parseInt(idStr, 10); | ||
this.type = | ||
typeStr === 'table' ? DatasourceType.Table : DatasourceType.Druid; | ||
this.type = typeStr === 'query' ? DatasourceType.Query : this.type; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this into a switch statement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can remove DatasourceType.Druid |
||
} | ||
|
||
public toString() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,11 +47,17 @@ export interface Datasource { | |
}; | ||
} | ||
|
||
export const DEFAULT_METRICS = [ | ||
export const DEFAULT_METRICS: Metric[] = [ | ||
{ | ||
metric_name: 'COUNT(*)', | ||
expression: 'COUNT(*)', | ||
}, | ||
]; | ||
|
||
export const isValidDatasourceType = (datasource: DatasourceType) => | ||
datasource === DatasourceType.Dataset || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we do something like |
||
datasource === DatasourceType.SlTable || | ||
datasource === DatasourceType.SavedQuery || | ||
datasource === DatasourceType.Query; | ||
|
||
export default {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,11 @@ import Button from 'src/components/Button'; | |
import shortid from 'shortid'; | ||
import { styled, t, QueryResponse } from '@superset-ui/core'; | ||
import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; | ||
import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal'; | ||
import { | ||
ISaveableDataset, | ||
ISimpleColumn, | ||
SaveDatasetModal, | ||
} from 'src/SqlLab/components/SaveDatasetModal'; | ||
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; | ||
import ProgressBar from 'src/components/ProgressBar'; | ||
import Loading from 'src/components/Loading'; | ||
|
@@ -220,6 +224,15 @@ export default class ResultSet extends React.PureComponent< | |
const { showSaveDatasetModal } = this.state; | ||
const { query } = this.props; | ||
|
||
const dataset: ISaveableDataset = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eric-briscoe what does the convention of naming with a prefix of |
||
columns: query.columns as ISimpleColumn[], | ||
name: query?.tab || 'Untitled', | ||
dbId: 1, | ||
sql: query.sql, | ||
templateParams: query.templateParams, | ||
schema: query.schema, | ||
}; | ||
|
||
return ( | ||
<ResultSetControls> | ||
<SaveDatasetModal | ||
|
@@ -230,14 +243,24 @@ export default class ResultSet extends React.PureComponent< | |
modalDescription={t( | ||
'Save this query as a virtual dataset to continue exploring', | ||
)} | ||
datasource={query} | ||
datasource={dataset} | ||
/> | ||
<ResultSetButtons> | ||
{this.props.visualize && | ||
this.props.database?.allows_virtual_table_explore && ( | ||
<ExploreResultsButton | ||
database={this.props.database} | ||
onClick={() => this.setState({ showSaveDatasetModal: true })} | ||
onClick={() => { | ||
// There is currently redux / state issue where sometimes a query will have serverId | ||
// and other times it will not. We need this attribute consistently for this to work | ||
const qid = this.props?.query?.results?.query_id; | ||
if (qid) { | ||
// This will open expolore using the query as datasource | ||
window.location.href = `/superset/explore/query/${qid}`; | ||
} else { | ||
this.setState({ showSaveDatasetModal: true }); | ||
} | ||
}} | ||
/> | ||
)} | ||
{this.props.csv && ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,17 +45,41 @@ import { | |
DatasetOptionAutocomplete, | ||
SqlLabExploreRootState, | ||
getInitialState, | ||
ExploreDatasource, | ||
} from 'src/SqlLab/types'; | ||
|
||
import { exploreChart } from 'src/explore/exploreUtils'; | ||
import { isString } from 'lodash'; | ||
|
||
interface QueryDatabase { | ||
id?: number; | ||
} | ||
|
||
export type ExploreQuery = QueryResponse & { | ||
database?: QueryDatabase | null | undefined; | ||
}; | ||
|
||
export interface ISimpleColumn { | ||
name?: string | null; | ||
type?: string | null; | ||
is_dttm?: boolean | null; | ||
} | ||
|
||
export interface ISaveableDataset { | ||
columns: ISimpleColumn[]; | ||
name: string; | ||
dbId: number; | ||
sql: string; | ||
templateParams?: string | object | null; | ||
schema?: string | null; | ||
} | ||
|
||
interface SaveDatasetModalProps { | ||
visible: boolean; | ||
onHide: () => void; | ||
buttonTextOnSave: string; | ||
buttonTextOnOverwrite: string; | ||
modalDescription?: string; | ||
datasource: ExploreDatasource; | ||
datasource: ISaveableDataset; | ||
} | ||
|
||
const Styles = styled.div` | ||
|
@@ -106,6 +130,8 @@ const updateDataset = async ( | |
return data.json.result; | ||
}; | ||
|
||
const UNTITLED = t('Untitled Dataset'); | ||
|
||
// eslint-disable-next-line no-empty-pattern | ||
export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({ | ||
visible, | ||
|
@@ -115,9 +141,8 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({ | |
modalDescription, | ||
datasource, | ||
}) => { | ||
const query = datasource as QueryResponse; | ||
const getDefaultDatasetName = () => | ||
`${query.tab} ${moment().format('MM/DD/YYYY HH:mm:ss')}`; | ||
`${datasource?.name || UNTITLED} ${moment().format('MM/DD/YYYY HH:mm:ss')}`; | ||
const [datasetName, setDatasetName] = useState(getDefaultDatasetName()); | ||
const [newOrOverwrite, setNewOrOverwrite] = useState( | ||
DatasetRadioState.SAVE_NEW, | ||
|
@@ -138,10 +163,10 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({ | |
|
||
const handleOverwriteDataset = async () => { | ||
await updateDataset( | ||
query.dbId, | ||
datasource.dbId, | ||
datasetToOverwrite.datasetId, | ||
query.sql, | ||
query.results.selected_columns.map( | ||
datasource.sql, | ||
datasource?.columns?.map( | ||
(d: { name: string; type: string; is_dttm: boolean }) => ({ | ||
column_name: d.name, | ||
type: d.type, | ||
|
@@ -159,7 +184,8 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({ | |
exploreChart({ | ||
...EXPLORE_CHART_DEFAULT, | ||
datasource: `${datasetToOverwrite.datasetId}__table`, | ||
selected_columns: query.results.selected_columns, | ||
all_columns: datasource?.columns?.map?.((d: ISimpleColumn) => d.name), | ||
selected_columns: datasource?.columns, | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we remove this commented out code? |
||
}; | ||
|
||
|
@@ -209,28 +235,29 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({ | |
return; | ||
} | ||
|
||
const selectedColumns = query.results.selected_columns || []; | ||
const selectedColumns = datasource?.columns ?? []; | ||
|
||
// The filters param is only used to test jinja templates. | ||
// Remove the special filters entry from the templateParams | ||
// before saving the dataset. | ||
if (query.templateParams) { | ||
const p = JSON.parse(query.templateParams); | ||
let templateParams; | ||
if (isString(datasource.templateParams)) { | ||
const p = JSON.parse(datasource.templateParams); | ||
/* eslint-disable-next-line no-underscore-dangle */ | ||
if (p._filters) { | ||
/* eslint-disable-next-line no-underscore-dangle */ | ||
delete p._filters; | ||
// eslint-disable-next-line no-param-reassign | ||
query.templateParams = JSON.stringify(p); | ||
templateParams = JSON.stringify(p); | ||
} | ||
} | ||
|
||
dispatch( | ||
createDatasource({ | ||
schema: query.schema, | ||
sql: query.sql, | ||
dbId: query.dbId, | ||
templateParams: query.templateParams, | ||
schema: datasource.schema, | ||
sql: datasource.sql, | ||
dbId: datasource.dbId, | ||
templateParams, | ||
datasourceName: datasetName, | ||
columns: selectedColumns, | ||
}), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this druid reference still in master? cc @AAfghahi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bump @AAfghahi or @hughhhh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea it's still there going to remove it now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Sorry, I just saw this, and yes it is still in master. We should change it here.