villebro commented on code in PR #35077:
URL: https://github.com/apache/superset/pull/35077#discussion_r2337135400
##########
superset-frontend/packages/superset-core/src/api/sqlLab.ts:
##########
@@ -111,6 +111,171 @@ export interface Tab {
panels: Panel[];
}
+/**
+ * Generic data types, see enum of the same name in superset/utils/core.py.
+ */
+enum GenericDataType {
+ Numeric = 0,
+ String = 1,
+ Temporal = 2,
+ Boolean = 3,
+}
+
+/**
+ * Column metadata returned in query results.
+ */
+export type QueryColumn = {
+ /**
+ * Label of the column
+ */
+ name?: string;
+
+ /**
+ * Column name defined
+ */
+ column_name: string;
+
+ /**
+ * Type of the column value
+ */
+ type: string | null;
+
+ /**
+ * Generic data type format
+ */
+ type_generic: GenericDataType;
+
+ /**
+ * True if the column is date format
+ */
+ is_dttm: boolean;
+};
+
+export enum CTASMethod {
+ Table = 'TABLE',
+ View = 'VIEW',
+}
+
+export interface CTAS {
+ /**
+ * Create method for CTAS creation request
+ */
+ method: CTASMethod;
+
+ /**
+ * Temporary table name for creation using a CTAS query
+ */
+ tempTable: string | null;
+}
+
+export interface QueryRequestContext {
Review Comment:
We actually already have a `QueryContext` class in the backend that's used
by the chart data API. So let's make sure we disambiguate those.
##########
superset-frontend/packages/superset-core/src/api/core.ts:
##########
@@ -76,6 +76,35 @@ export declare interface Database {
schemas: Schema[];
}
+// Keep in sync with superset/errors.py
+export type ErrorLevel = 'info' | 'warning' | 'error';
Review Comment:
Yeah, now/soon may be the time to start considering something like this, as
keeping these types synced between FE/BE is pretty error prone.
##########
superset-frontend/packages/superset-core/src/api/sqlLab.ts:
##########
@@ -111,6 +111,171 @@ export interface Tab {
panels: Panel[];
}
+/**
+ * Generic data types, see enum of the same name in superset/utils/core.py.
+ */
+enum GenericDataType {
Review Comment:
I suspect we'll soonish want to start replacing the JSON-based data blob
with a more performant binary format that has built-in data types. I'm thinking
we would want to default to using the binary transport protocol, but let
charts/extensions opt-in for the legacy JSON format, in which case they get the
generic data types. Just something to keep in mind for the migration of
`GenericDataType` to core..
##########
superset-frontend/packages/superset-core/src/api/sqlLab.ts:
##########
@@ -111,6 +111,171 @@ export interface Tab {
panels: Panel[];
}
+/**
+ * Generic data types, see enum of the same name in superset/utils/core.py.
+ */
+enum GenericDataType {
+ Numeric = 0,
+ String = 1,
+ Temporal = 2,
+ Boolean = 3,
+}
+
+/**
+ * Column metadata returned in query results.
+ */
+export type QueryColumn = {
+ /**
+ * Label of the column
+ */
+ name?: string;
+
+ /**
+ * Column name defined
+ */
+ column_name: string;
+
+ /**
+ * Type of the column value
+ */
+ type: string | null;
+
+ /**
+ * Generic data type format
+ */
+ type_generic: GenericDataType;
+
+ /**
+ * True if the column is date format
+ */
+ is_dttm: boolean;
+};
+
+export enum CTASMethod {
+ Table = 'TABLE',
+ View = 'VIEW',
+}
+
+export interface CTAS {
+ /**
+ * Create method for CTAS creation request
+ */
+ method: CTASMethod;
+
+ /**
+ * Temporary table name for creation using a CTAS query
+ */
+ tempTable: string | null;
+}
+
+export interface QueryRequestContext {
+ /**
+ * Unique query ID on client side
+ */
+ id: string;
+
+ /**
+ * Contains CTAS if the query requests table creation
+ */
+ ctas: CTAS | null;
+
+ /**
+ * The SQL editor instance associated with the query
+ */
+ editor: Editor;
+
+ /**
+ * Requested row limit for the query
+ */
+ queryLimit: number | null;
+
+ /**
+ * True if the query execution result will be/was delivered asynchronously
+ */
+ runAsync?: boolean;
+
+ /**
+ * Start datetime for the query in a numerical timestamp
+ */
+ startDttm: number;
+
+ /**
+ * The tab instance associated with the request query
+ */
+ tab: Tab;
+
+ /**
+ * A key-value JSON formatted string associated with Jinja template variables
+ */
+ templateParams: string;
+}
+
+export interface QueryErrorResultContext extends QueryRequestContext {
+ /**
+ * Finished datetime for the query in a numerical timestamp
+ */
+ endDttm: number;
+
+ /**
+ * Error message returned from DB engine
+ */
+ errorMessage: string;
+
+ /**
+ * Error details in a SupersetError structure
+ */
+ errors?: SupersetError[];
+
+ /**
+ * Executed sql after parsing the jinja templates
+ */
+ executedSql: string | null;
+}
+
+export interface QueryResultContext extends QueryRequestContext {
+ /**
+ * Finished datetime for the query in a numerical timestamp
+ */
+ endDttm: number;
+
+ /**
+ * Executed sql after parsing the jinja templates
+ */
+ executedSql: string;
+
+ /**
+ * Actual number of rows returned by the query
+ */
+ limit: number;
+
+ /**
+ * Major factor that is determining the row limit of the query results
+ */
+ limitingFactor: string;
+
+ /**
+ * Remote query id stored in backend
+ */
+ queryId: number;
Review Comment:
Same here - changing this would be a breaking change.
##########
superset-frontend/packages/superset-core/src/api/sqlLab.ts:
##########
@@ -111,6 +111,171 @@ export interface Tab {
panels: Panel[];
}
+/**
+ * Generic data types, see enum of the same name in superset/utils/core.py.
+ */
+enum GenericDataType {
+ Numeric = 0,
+ String = 1,
+ Temporal = 2,
+ Boolean = 3,
+}
+
+/**
+ * Column metadata returned in query results.
+ */
+export type QueryColumn = {
+ /**
+ * Label of the column
+ */
+ name?: string;
+
+ /**
+ * Column name defined
+ */
+ column_name: string;
Review Comment:
Automatic translation between snake/camel case can be tricky as has been
seen in the current charting implementation, in which there's `formData` with
converted camel to snake and `rawFormData` with the unconverted properties.
This has lead to inconsistent use of `formData` and `rawFormData`, with the
risk of collisions. If we want to pass along SQLA model instances via the REST
API, then it feels awkward to convert from the native format (snake_case) to
the local format (camelCase) just for the sake of aesthetics. So let's make
sure we think hard about language specific conventions vs having a single case
definition throughout the platform.
##########
superset-frontend/packages/superset-core/src/api/sqlLab.ts:
##########
@@ -111,6 +111,171 @@ export interface Tab {
panels: Panel[];
}
+/**
+ * Generic data types, see enum of the same name in superset/utils/core.py.
+ */
+enum GenericDataType {
+ Numeric = 0,
+ String = 1,
+ Temporal = 2,
+ Boolean = 3,
+}
+
+/**
+ * Column metadata returned in query results.
+ */
+export type QueryColumn = {
+ /**
+ * Label of the column
+ */
+ name?: string;
+
+ /**
+ * Column name defined
+ */
+ column_name: string;
+
+ /**
+ * Type of the column value
+ */
+ type: string | null;
+
+ /**
+ * Generic data type format
+ */
+ type_generic: GenericDataType;
+
+ /**
+ * True if the column is date format
+ */
+ is_dttm: boolean;
+};
+
+export enum CTASMethod {
+ Table = 'TABLE',
+ View = 'VIEW',
+}
+
+export interface CTAS {
+ /**
+ * Create method for CTAS creation request
+ */
+ method: CTASMethod;
+
+ /**
+ * Temporary table name for creation using a CTAS query
+ */
+ tempTable: string | null;
+}
+
+export interface QueryRequestContext {
+ /**
+ * Unique query ID on client side
+ */
+ id: string;
+
+ /**
+ * Contains CTAS if the query requests table creation
+ */
+ ctas: CTAS | null;
+
+ /**
+ * The SQL editor instance associated with the query
+ */
+ editor: Editor;
+
+ /**
+ * Requested row limit for the query
+ */
+ queryLimit: number | null;
+
+ /**
+ * True if the query execution result will be/was delivered asynchronously
+ */
+ runAsync?: boolean;
+
+ /**
+ * Start datetime for the query in a numerical timestamp
+ */
+ startDttm: number;
+
+ /**
+ * The tab instance associated with the request query
+ */
+ tab: Tab;
+
+ /**
+ * A key-value JSON formatted string associated with Jinja template variables
+ */
+ templateParams: string;
Review Comment:
I agree, while these are stored in string format in the SQLA model, these
should be deserialized by the backend when returning the result. However, since
this in the current implementation this is indeed a string, we will need to
wait for a breaking window to change this. Maybe in the interim introducing
`templateParameters: Record<string, any>` that is deserialized and deprecating
`templateParams`?
--
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]
