From a29f86152f8405ae9c4a3b9da56236f60c1a7a42 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Thu, 14 Jul 2022 20:04:10 +0530 Subject: [PATCH 1/3] wip: exclude unnecessary fields from select query Signed-off-by: Pranav C --- .../sql-data-mapper/lib/sql/BaseModelSqlv2.ts | 115 ++++++++++++------ 1 file changed, 77 insertions(+), 38 deletions(-) diff --git a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts index 6c20ec4089..0573639a4b 100644 --- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts +++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts @@ -1,47 +1,49 @@ import autoBind from 'auto-bind'; -import _ from 'lodash'; - -import Model from '../../../../models/Model'; -import { XKnex } from '../../index'; -import LinkToAnotherRecordColumn from '../../../../models/LinkToAnotherRecordColumn'; -import RollupColumn from '../../../../models/RollupColumn'; -import LookupColumn from '../../../../models/LookupColumn'; import DataLoader from 'dataloader'; -import Column from '../../../../models/Column'; -import { XcFilter, XcFilterWithAlias } from '../BaseModel'; -import conditionV2 from './conditionV2'; -import Filter from '../../../../models/Filter'; -import sortV2 from './sortV2'; -import Sort from '../../../../models/Sort'; -import FormulaColumn from '../../../../models/FormulaColumn'; -import genRollupSelectv2 from './genRollupSelectv2'; -import formulaQueryBuilderv2 from './formulav2/formulaQueryBuilderv2'; +import ejs from 'ejs'; +import DOMPurify from 'isomorphic-dompurify'; import { QueryBuilder } from 'knex'; -import View from '../../../../models/View'; +import _ from 'lodash'; +import { customAlphabet } from 'nanoid'; import { AuditOperationSubTypes, AuditOperationTypes, + isSystemColumn, RelationTypes, SortType, UITypes, ViewTypes, } from 'nocodb-sdk'; -import formSubmissionEmailTemplate from '../../../../utils/common/formSubmissionEmailTemplate'; -import ejs from 'ejs'; -import Audit from '../../../../models/Audit'; -import FormView from '../../../../models/FormView'; -import Hook from '../../../../models/Hook'; +import Validator from 'validator'; +import { NcError } from '../../../../meta/helpers/catchError'; import NcPluginMgrv2 from '../../../../meta/helpers/NcPluginMgrv2'; import { _transformSubmittedFormDataForEmail, invokeWebhook, } from '../../../../meta/helpers/webhookHelpers'; -import Validator from 'validator'; +import Audit from '../../../../models/Audit'; +import Column from '../../../../models/Column'; +import Filter from '../../../../models/Filter'; +import FormulaColumn from '../../../../models/FormulaColumn'; +import FormView from '../../../../models/FormView'; +import GridViewColumn from '../../../../models/GridViewColumn'; +import Hook from '../../../../models/Hook'; +import LinkToAnotherRecordColumn from '../../../../models/LinkToAnotherRecordColumn'; +import LookupColumn from '../../../../models/LookupColumn'; + +import Model from '../../../../models/Model'; +import RollupColumn from '../../../../models/RollupColumn'; +import Sort from '../../../../models/Sort'; +import View from '../../../../models/View'; +import formSubmissionEmailTemplate from '../../../../utils/common/formSubmissionEmailTemplate'; +import { XKnex } from '../../index'; +import { XcFilter, XcFilterWithAlias } from '../BaseModel'; +import conditionV2 from './conditionV2'; import { customValidators } from './customValidators'; -import { NcError } from '../../../../meta/helpers/catchError'; -import { customAlphabet } from 'nanoid'; -import DOMPurify from 'isomorphic-dompurify'; +import formulaQueryBuilderv2 from './formulav2/formulaQueryBuilderv2'; +import genRollupSelectv2 from './genRollupSelectv2'; import { sanitize, unsanitize } from './helpers/sanitize'; +import sortV2 from './sortV2'; const GROUP_COL = '__nc_group_id'; @@ -57,6 +59,15 @@ async function populatePk(model: Model, insertObj: any) { } } +function checkColumnRequired(column: Column, fields: string[]) { + // if primary key or foreign key included in fields, it's required + if (column.pk || column.uidt === UITypes.ForeignKey) return true; + + // check fields defined and if not, then select all + // if defined check if it is in the fields + return !fields || fields.includes(column.title); +} + /** * Base class for models * @@ -175,10 +186,10 @@ class BaseModelSqlv2 { } = {}, ignoreFilterSort = false ): Promise { - const { where, ...rest } = this._getListArgs(args as any); + const { where, fields, ...rest } = this._getListArgs(args as any); const qb = this.dbDriver(this.tnPath); - await this.selectObject({ qb }); + await this.selectObject({ qb, fields }); if (+rest?.shuffle) { await this.shuffle({ qb }); } @@ -251,7 +262,9 @@ class BaseModelSqlv2 { if (!ignoreFilterSort) applyPaginate(qb, rest); const proto = await this.getProto(); - let data = await this.extractRawQueryAndExec(qb); + const data = await this.extractRawQueryAndExec(qb); + + console.log(qb.toQuery()); return data?.map((d) => { d.__proto__ = proto; @@ -362,7 +375,7 @@ class BaseModelSqlv2 { qb.groupBy(args.column_name); if (sorts) await sortV2(sorts, qb, this.dbDriver); applyPaginate(qb, rest); - let data = await qb; + const data = await qb; return data; } @@ -571,7 +584,10 @@ class BaseModelSqlv2 { } } - public async multipleMmList({ colId, parentIds }, args: { limit?; offset? } = {}) { + public async multipleMmList( + { colId, parentIds }, + args: { limit?; offset? } = {} + ) { const { where, ...rest } = this._getListArgs(args as any); const relColumn = (await this.model.getColumns()).find( (c) => c.id === colId @@ -879,7 +895,7 @@ class BaseModelSqlv2 { applyPaginate(qb, rest); const proto = await childModel.getProto(); - let data = await qb; + const data = await qb; return data.map((c) => { c.__proto__ = proto; @@ -979,7 +995,7 @@ class BaseModelSqlv2 { applyPaginate(qb, rest); const proto = await childModel.getProto(); - let data = await this.extractRawQueryAndExec(qb); + const data = await this.extractRawQueryAndExec(qb); return data.map((c) => { c.__proto__ = proto; @@ -1079,7 +1095,7 @@ class BaseModelSqlv2 { applyPaginate(qb, rest); const proto = await parentModel.getProto(); - let data = await this.extractRawQueryAndExec(qb); + const data = await this.extractRawQueryAndExec(qb); return data.map((c) => { c.__proto__ = proto; @@ -1272,7 +1288,7 @@ class BaseModelSqlv2 { this.config.limitMin ); obj.offset = Math.max(+(args.offset || args.o) || 0, 0); - obj.fields = args.fields || args.f || '*'; + obj.fields = args.fields || args.f; obj.sort = args.sort || args.s || this.model.primaryKey?.[0]?.tn; return obj; } @@ -1287,10 +1303,33 @@ class BaseModelSqlv2 { } } - public async selectObject({ qb }: { qb: QueryBuilder }): Promise { + public async selectObject({ + qb, + fields: _fields, + }: { + qb: QueryBuilder; + fields?: string[] | string; + }): Promise { + const view = await View.get(this.viewId); + const viewColumns = this.viewId && (await View.getColumns(this.viewId)); + const fields = Array.isArray(_fields) ? _fields : _fields?.split(','); const res = {}; - const columns = await this.model.getColumns(); - for (const column of columns) { + const columns = viewColumns || (await this.model.getColumns()); + for (const _column of columns) { + const column = + _column instanceof Column + ? _column + : await Column.get({ + colId: (_column as GridViewColumn).fk_column_id, + }); + if ( + !(column instanceof Column) && + !(column as GridViewColumn)?.show && + (!view?.show_system_fields || isSystemColumn(column)) + ) + continue; + if (!checkColumnRequired(column, fields)) continue; + switch (column.uidt) { case 'LinkToAnotherRecord': case 'Lookup': From 91b86b04e42c4685fb173683342a326338274eb8 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Thu, 14 Jul 2022 23:23:17 +0530 Subject: [PATCH 2/3] feat: exclude based on view and fields property - exclude columns in root list query based on filed param and view - include foreign key and primary keys by default Signed-off-by: Pranav C --- .../sql-data-mapper/lib/sql/BaseModelSqlv2.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts index 0573639a4b..f32866528a 100644 --- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts +++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts @@ -1314,18 +1314,22 @@ class BaseModelSqlv2 { const viewColumns = this.viewId && (await View.getColumns(this.viewId)); const fields = Array.isArray(_fields) ? _fields : _fields?.split(','); const res = {}; - const columns = viewColumns || (await this.model.getColumns()); - for (const _column of columns) { + const viewOrTableColumns = viewColumns || (await this.model.getColumns()); + for (const viewOrTableColumn of viewOrTableColumns) { const column = - _column instanceof Column - ? _column + viewOrTableColumn instanceof Column + ? viewOrTableColumn : await Column.get({ - colId: (_column as GridViewColumn).fk_column_id, + colId: (viewOrTableColumn as GridViewColumn).fk_column_id, }); + // hide if column marked as hidden in view + // of if column is system field and system field is hidden if ( - !(column instanceof Column) && - !(column as GridViewColumn)?.show && - (!view?.show_system_fields || isSystemColumn(column)) + !(viewOrTableColumn instanceof Column) && + (!(viewOrTableColumn as GridViewColumn)?.show || + (!view?.show_system_fields && + column.uidt !== UITypes.ForeignKey && + isSystemColumn(column))) ) continue; if (!checkColumnRequired(column, fields)) continue; From 4f4a02e7863d021a075a7b36c93c718e7e0acf08 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Thu, 14 Jul 2022 23:30:30 +0530 Subject: [PATCH 3/3] wip: extract dependency columns Signed-off-by: Pranav C --- .../sql-data-mapper/lib/sql/BaseModelSqlv2.ts | 34 ++++- .../sql-data-mapper/lib/sql/helpers/getAst.ts | 133 ++++++++++++++++-- .../lib/meta/api/dataApis/dataAliasApis.ts | 21 ++- packages/nocodb/src/lib/models/Column.ts | 2 +- 4 files changed, 165 insertions(+), 25 deletions(-) diff --git a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts index f32866528a..548603195b 100644 --- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts +++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts @@ -59,10 +59,16 @@ async function populatePk(model: Model, insertObj: any) { } } -function checkColumnRequired(column: Column, fields: string[]) { +function checkColumnRequired( + column: Column, + fields: string[], + extractPkAndPv?: boolean +) { // if primary key or foreign key included in fields, it's required if (column.pk || column.uidt === UITypes.ForeignKey) return true; + if (extractPkAndPv && column.pv) return true; + // check fields defined and if not, then select all // if defined check if it is in the fields return !fields || fields.includes(column.title); @@ -189,7 +195,7 @@ class BaseModelSqlv2 { const { where, fields, ...rest } = this._getListArgs(args as any); const qb = this.dbDriver(this.tnPath); - await this.selectObject({ qb, fields }); + await this.selectObject({ qb, fields, viewId: this.viewId }); if (+rest?.shuffle) { await this.shuffle({ qb }); } @@ -264,7 +270,7 @@ class BaseModelSqlv2 { const proto = await this.getProto(); const data = await this.extractRawQueryAndExec(qb); - console.log(qb.toQuery()); + // console.log(qb.toQuery()); return data?.map((d) => { d.__proto__ = proto; @@ -404,7 +410,7 @@ class BaseModelSqlv2 { await parentTable.getColumns(); const qb = this.dbDriver(childTable.table_name); - await childModel.selectObject({ qb }); + await childModel.selectObject({ qb, extractPkAndPv: true }); const childQb = this.dbDriver.queryBuilder().from( this.dbDriver @@ -431,6 +437,9 @@ class BaseModelSqlv2 { .as('list') ); + + // console.log(childQb.toQuery()) + const children = await this.extractRawQueryAndExec(childQb); const proto = await ( await Model.getBaseModelSQL({ @@ -439,6 +448,7 @@ class BaseModelSqlv2 { }) ).getProto(); + return _.groupBy( children.map((c) => { c.__proto__ = proto; @@ -1303,15 +1313,22 @@ class BaseModelSqlv2 { } } + // todo: + // pass view id as argument + // add option to get only pk and pv public async selectObject({ qb, fields: _fields, + extractPkAndPv, + viewId, }: { qb: QueryBuilder; fields?: string[] | string; + extractPkAndPv?: boolean; + viewId?: string; }): Promise { - const view = await View.get(this.viewId); - const viewColumns = this.viewId && (await View.getColumns(this.viewId)); + const view = await View.get(viewId); + const viewColumns = viewId && (await View.getColumns(viewId)); const fields = Array.isArray(_fields) ? _fields : _fields?.split(','); const res = {}; const viewOrTableColumns = viewColumns || (await this.model.getColumns()); @@ -1325,14 +1342,17 @@ class BaseModelSqlv2 { // hide if column marked as hidden in view // of if column is system field and system field is hidden if ( + !extractPkAndPv && !(viewOrTableColumn instanceof Column) && (!(viewOrTableColumn as GridViewColumn)?.show || (!view?.show_system_fields && column.uidt !== UITypes.ForeignKey && + !column.pk && isSystemColumn(column))) ) continue; - if (!checkColumnRequired(column, fields)) continue; + + if (!checkColumnRequired(column, fields, extractPkAndPv)) continue; switch (column.uidt) { case 'LinkToAnotherRecord': diff --git a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/helpers/getAst.ts b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/helpers/getAst.ts index 9e6af05f6e..6875349300 100644 --- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/helpers/getAst.ts +++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/helpers/getAst.ts @@ -1,7 +1,9 @@ -import View from '../../../../../models/View'; -import { isSystemColumn, UITypes } from 'nocodb-sdk'; -import Model from '../../../../../models/Model'; +import { isSystemColumn, RelationTypes, UITypes } from 'nocodb-sdk'; +import Column from '../../../../../models/Column'; import LinkToAnotherRecordColumn from '../../../../../models/LinkToAnotherRecordColumn'; +import LookupColumn from '../../../../../models/LookupColumn'; +import Model from '../../../../../models/Model'; +import View from '../../../../../models/View'; const getAst = async ({ query, @@ -9,23 +11,34 @@ const getAst = async ({ includePkByDefault = true, model, view, + dependencyFields = { + nested: {}, + fields: new Set(), + }, }: { query?: RequestQuery; extractOnlyPrimaries?: boolean; includePkByDefault?: boolean; model: Model; view?: View; + dependencyFields?: DependantFields; }) => { if (!model.columns?.length) await model.getColumns(); // extract only pk and pv if (extractOnlyPrimaries) { - return { + const ast = { ...(model.primaryKeys ? model.primaryKeys.reduce((o, pk) => ({ ...o, [pk.title]: 1 }), {}) : {}), ...(model.primaryValue ? { [model.primaryValue.title]: 1 } : {}), }; + await Promise.all( + model.primaryKeys.map((c) => extractDependencies(c, dependencyFields)) + ); + await extractDependencies(model.primaryValue, dependencyFields); + + return { ast, dependencyFields }; } let fields = query?.fields || query?.f; @@ -45,7 +58,7 @@ const getAst = async ({ {} ); - return model.columns.reduce(async (obj, col) => { + const ast = await model.columns.reduce(async (obj, col) => { let value: number | boolean | { [key: string]: any } = 1; const nestedFields = query?.nested?.[col.title]?.fields || query?.nested?.[col.title]?.f; @@ -55,10 +68,19 @@ const getAst = async ({ .getColOptions() .then((colOpt) => colOpt.getRelatedTable()); - value = await getAst({ + const { ast } = await getAst({ model, query: query?.nested?.[col.title], + dependencyFields: (dependencyFields.nested[col.title] = + dependencyFields.nested[col.title] || { + nested: {}, + fields: new Set(), + }), }); + + value = ast; + + // todo: include field relative to the relation => pk / fk } else { value = (Array.isArray(fields) ? fields : fields.split(',')).reduce( (o, f) => ({ ...o, [f]: 1 }), @@ -74,22 +96,98 @@ const getAst = async ({ model, query: query?.nested?.[col.title], extractOnlyPrimaries: nestedFields !== '*', + dependencyFields:(dependencyFields.nested[col.title] = + dependencyFields.nested[col.title] || { + nested: {}, + fields: new Set(), + }) }); } + const isRequested = + allowedCols && (!includePkByDefault || !col.pk) + ? allowedCols[col.id] && + (!isSystemColumn(col) || view.show_system_fields) && + (!fields?.length || fields.includes(col.title)) && + value + : fields?.length + ? fields.includes(col.title) && value + : value; + if (isRequested) await extractDependencies(col, dependencyFields); + return { ...(await obj), - [col.title]: - allowedCols && (!includePkByDefault || !col.pk) - ? allowedCols[col.id] && - (!isSystemColumn(col) || view.show_system_fields) && - (!fields?.length || fields.includes(col.title)) && - value - : fields?.length - ? fields.includes(col.title) && value - : value, + [col.title]: isRequested, }; }, Promise.resolve({})); + + return { ast, dependencyFields }; +}; + +const extractDependencies = async ( + column: Column, + dependencyFields: DependantFields = { + nested: {}, + fields: new Set(), + } +) => { + switch (column.uidt) { + case UITypes.Lookup: + await extractLookupDependencies(column, dependencyFields); + break; + case UITypes.LinkToAnotherRecord: + await extractRelationDependencies(column, dependencyFields); + break; + default: + dependencyFields.fields.add(column.title); + break; + } +}; + +const extractLookupDependencies = async ( + lookUpColumn: Column, + dependencyFields: DependantFields = { + nested: {}, + fields: new Set(), + } +) => { + const lookupColumnOpts = await lookUpColumn.getColOptions(); + const relationColumn = await lookupColumnOpts.getRelationColumn(); + await extractRelationDependencies(relationColumn, dependencyFields); + await extractDependencies( + await lookupColumnOpts.getLookupColumn(), + (dependencyFields.nested[relationColumn.title] = dependencyFields.nested[ + relationColumn.title + ] || { + nested: {}, + fields: new Set(), + }) + ); +}; + +const extractRelationDependencies = async ( + relationColumn: Column, + dependencyFields: DependantFields = { + nested: {}, + fields: new Set(), + } +) => { + const relationColumnOpts = await relationColumn.getColOptions(); + + switch (relationColumnOpts.type) { + case RelationTypes.HAS_MANY: + dependencyFields.fields.add( + await relationColumnOpts.getParentColumn().then((col) => col.title) + ); + break; + case RelationTypes.BELONGS_TO: + case RelationTypes.MANY_TO_MANY: + dependencyFields.fields.add( + await relationColumnOpts.getChildColumn().then((col) => col.title) + ); + + break; + } }; type RequestQuery = { @@ -100,4 +198,9 @@ type RequestQuery = { }; }; +interface DependantFields { + fields?: Set; + nested?: DependantFields; +} + export default getAst; diff --git a/packages/nocodb/src/lib/meta/api/dataApis/dataAliasApis.ts b/packages/nocodb/src/lib/meta/api/dataApis/dataAliasApis.ts index b982bbc4de..40ce3e782b 100644 --- a/packages/nocodb/src/lib/meta/api/dataApis/dataAliasApis.ts +++ b/packages/nocodb/src/lib/meta/api/dataApis/dataAliasApis.ts @@ -97,7 +97,11 @@ async function getDataList(model, view: View, req) { dbDriver: NcConnectionMgrv2.get(base), }); - const requestObj = await getAst({ model, query: req.query, view }); + const { ast, dependencyFields } = await getAst({ + model, + query: req.query, + view, + }); const listArgs: any = { ...req.query }; try { @@ -107,8 +111,21 @@ async function getDataList(model, view: View, req) { listArgs.sortArr = JSON.parse(listArgs.sortArrJson); } catch (e) {} + console.log( + JSON.stringify( + dependencyFields, + (_v, o) => { + if (o instanceof Set) { + return [...o]; + } + return o; + }, + 2 + ) + ); + console.log(JSON.stringify(ast, null, 2)); const data = await nocoExecute( - requestObj, + ast, await baseModel.list(listArgs), {}, listArgs diff --git a/packages/nocodb/src/lib/models/Column.ts b/packages/nocodb/src/lib/models/Column.ts index 5589888ef3..96140f6c15 100644 --- a/packages/nocodb/src/lib/models/Column.ts +++ b/packages/nocodb/src/lib/models/Column.ts @@ -308,7 +308,7 @@ export default class Column implements ColumnType { } } - public async getColOptions(ncMeta = Noco.ncMeta): Promise { + public async getColOptions(ncMeta = Noco.ncMeta): Promise { let res: any; switch (this.uidt) {