Skip to content
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: export queryFieldInfo and jsType #625

Merged
merged 8 commits into from
Dec 13, 2024
Merged
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
5 changes: 3 additions & 2 deletions packages/core/src/Coordinator.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function coordinator(instance) {
}

/**
* @typedef {import('@uwdata/mosaic-sql').Query | string} QueryType
* @typedef {import('@uwdata/mosaic-sql').Query | import('@uwdata/mosaic-sql').DescribeQuery | string} QueryType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DescribeQuery was missing as a QueryType causing an error

*/

/**
Expand Down Expand Up @@ -134,7 +134,8 @@ export class Coordinator {
* or a SQL string.
* @param {object} [options] An options object.
* @param {'arrow' | 'json'} [options.type] The query result format type.
* @param {boolean} [options.cache=true] If true, cache the query result.
* @param {boolean} [options.cache=true] If true, cache the query result in the client (QueryManager).
* @param {boolean} [options.persist=false] If true, persist cached query result in the server.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

persist option was missing in the type definition causing an error

Copy link
Member

Choose a reason for hiding this comment

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

Can you say more about where the error was coming from? I believe persist should be part of the aggregated ...options, not broken out individually.

Copy link
Contributor Author

@kwonoh kwonoh Dec 13, 2024

Choose a reason for hiding this comment

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

Can you say more about where the error was coming from?

The type error was from { persist: true } ingetFieldInfo. Currently, there is no persist field in the options type info of Coordinator.query here.

I believe persist should be part of the aggregated ...options, not broken out individually.

persist type info is added as a member of options object (options.persist not persist), not as a separate field. Do you want to add a new interface type definition for options separately?

or did you mean there should be no default value assignment for persist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now see what you mean. Updated the code.

* @param {number} [options.priority] The query priority, defaults to
* `Priority.Normal`.
* @returns {QueryResult} A query result promise.
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/MosaicClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ export class MosaicClient {

/**
* Return an array of fields queried by this client.
* @returns {object[]|null} The fields to retrieve info for.
* @returns {import('./util/field-info.js').FieldInfoRequest[] | null} The fields to retrieve info for.
*/
fields() {
return null;
}

/**
* Called by the coordinator to set the field info for this client.
* @param {*} info The field info result.
* @param {import('./util/field-info.js').FieldInfo[]} info The field info result.
* @returns {this}
*/
fieldInfo(info) { // eslint-disable-line no-unused-vars
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export { isArrowTable } from './util/is-arrow-table.js';
export { synchronizer } from './util/synchronizer.js';
export { throttle } from './util/throttle.js';
export { toDataColumns } from './util/to-data-columns.js';
export { queryFieldInfo } from './util/field-info.js';
export { jsType } from './util/js-type.js';

/**
* @typedef {import('./util/selection-types.js').ClauseMetadata} ClauseMetadata
Expand Down
64 changes: 52 additions & 12 deletions packages/core/src/util/field-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,25 @@ export const Distinct = 'distinct';
export const Stats = { Count, Nulls, Max, Min, Distinct };

/**
* @type {Record<string, (column: string) => AggregateNode>}
* @typedef {Count | Distinct | Max | Min | Nulls} Stat
*
* @typedef {{
* table: string | import('@uwdata/mosaic-sql').TableRefNode,
* column: (string | import('@uwdata/mosaic-sql').ColumnRefNode) & { aggregate?: boolean },
* stats?: Stat[] | Set<Stat>
* }} FieldInfoRequest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure FieldInfoRequest is a good name for a type that represents the return type of MosaicClient.fields and the type of fields param of queryFieldInfo

*
* @typedef {{
* table: FieldInfoRequest["table"],
* column: string,
Copy link
Contributor Author

@kwonoh kwonoh Dec 5, 2024

Choose a reason for hiding this comment

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

While FieldInfoRequest.column is string | ColumnRefNode, both getFieldInfo and getTableInfo return only string type for column field.

* sqlType: string,
* type: ReturnType<typeof jsType>,
* nullable: boolean,
* } & Partial<Record<Stat, number>>} FieldInfo
*/

/**
* @type {Record<Stat, (column: FieldInfoRequest["column"]) => AggregateNode>}
*/
const statMap = {
[Count]: count,
Expand All @@ -20,18 +38,26 @@ const statMap = {
};

/**
*
* @param {string} table
* @param {string} column
* @param {string[]|Set<string>} stats
* @returns
* Get summary stats of the given column
* @param {FieldInfoRequest} field
* @returns {import('@uwdata/mosaic-sql').Query}
*/
function summarize(table, column, stats) {
function summarize({ table, column, stats }) {
return Query
.from(table)
.select(Array.from(stats, s => ({[s]: statMap[s](column)})));
.select(Array.from(stats, s => ({ [s]: statMap[s](column) })));
}

/**
* Queries information about fields of a table.
* If the `fields` array contains a single field with the column set to '*',
* the function will retrieve and return the table information using `getTableInfo`.
* Otherwise, it will query individual field information using `getFieldInfo`
* for each field in the `fields` array.
* @param {import('../Coordinator.js').Coordinator} mc A Mosaic coordinator.
* @param {FieldInfoRequest[]} fields
* @returns {Promise<FieldInfo[]>}
*/
export async function queryFieldInfo(mc, fields) {
if (fields.length === 1 && fields[0].column === '*') {
return getTableInfo(mc, fields[0].table);
Expand All @@ -42,13 +68,20 @@ export async function queryFieldInfo(mc, fields) {
}
}

/**
* Get information about a single field of a table.
* @param {import('../Coordinator.js').Coordinator} mc A Mosaic coordinator.
* @param {FieldInfoRequest} field
* @returns {Promise<FieldInfo>}
*/
async function getFieldInfo(mc, { table, column, stats }) {
// generate and issue a query for field metadata info
// use GROUP BY ALL to differentiate & consolidate aggregates
const q = Query
.from({ source: table })
.select({ column })
.groupby(column.aggregate ? sql`ALL` : []);
/** @type {{ column_name: string, column_type: string, null: "YES" | "NO" }[]} */
const [desc] = Array.from(await mc.query(Query.describe(q)));
const info = {
table,
Expand All @@ -59,21 +92,28 @@ async function getFieldInfo(mc, { table, column, stats }) {
};

// no need for summary statistics
if (!(stats?.length || stats?.size)) return info;
if (!((stats instanceof Set && stats.size) || (Array.isArray(stats) && stats.length))) return info;

// query for summary stats
const [result] = await mc.query(
summarize(table, column, stats),
summarize({ table, column, stats }),
{ persist: true }
);

// extract summary stats, copy to field info, and return
return Object.assign(info, result);
}

/**
* Get information about the fields of a table.
* @param {import('../Coordinator.js').Coordinator} mc A Mosaic coordinator.
* @param {FieldInfoRequest["table"]} table the table name or reference
* @returns {Promise<FieldInfo[]>}
*/
async function getTableInfo(mc, table) {
const result = await mc.query(`DESCRIBE ${asTableRef(table)}`);
return Array.from(result).map(desc => ({
/** @type {{ column_name: string, column_type: string, null: "YES" | "NO" }[]} */
const result = Array.from(await mc.query(`DESCRIBE ${asTableRef(table)}`));
return result.map(desc => ({
table,
column: desc.column_name,
sqlType: desc.column_type,
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/util/js-type.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/**
* Maps a SQL data type to its corresponding JavaScript type.
* @param {string} type The name of a SQL data type
* @returns {"number" | "date" | "boolean" | "string" | "array" | "object"} The corresponding JavaScript type name
* @throws {Error} Throws an error if the given SQL type name is unsupported or unrecognized.
*/
export function jsType(type) {
switch (type) {
case 'BIGINT':
Expand Down
Loading