Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2026-03-02 04:09:24 +00:00
Code Issues Projects Releases Wiki Activity

(core) Store formula values in DB, and include them into .stored/.undo fields of actions.

Browse Source 
Summary:
- Introduce a new SQLiteDB migration, which adds DB columns for formula columns
- Newly added columns have the special ['P'] (pending) value in them
  (in order to show the usual "Loading..." on the first load that triggers the migration)
- Calculated values are added to .stored/.undo fields of user actions.
- Various changes made in the sandbox to include .stored/.undo in the right order.
- OnDemand tables ignore stored formula columns, replacing them with special SQL as before
- In particular, converting to OnDemand table leaves stale values in those
  columns, we should maybe clean those out.

Some tweaks on the side:
- Allow overriding chai assertion truncateThreshold with CHAI_TRUNCATE_THRESHOLD
- Rebuild python automatically in watch mode

Test Plan: Fixed various tests, updated some fixtures. Many python tests that check actions needed adjustments because actions moved from .stored to .undo. Some checks added to catch situations previously only caught in browser tests.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2645
This commit is contained in:
Dmitry S
2020-11-02 10:48:47 -05:00
parent 3d3fe92bd0
commit e2226c3ab7
33 changed files with 1188 additions and 553 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
app/server/lib/ActionHistory.ts
View File

@@ -31,6 +31,9 @@ export interface ActionGroupOptions {
// Values returned by the action, if known.
retValues?: any[];
// Set the 'internal' flag on the created actions, as inappropriate to undo.
internal?: boolean;
}
export abstract class ActionHistory {
@@ -259,8 +262,6 @@ export function asActionGroup(history: ActionHistory,
rowIdHint,
primaryAction,
isUndo,
internal: act.actionNum === 0 // Mark lazy-loading calculated columns. In future,
// synchronizing fields to today's date and other
// changes from external values may count as internal.
internal: options.internal || false,
};
}

9
app/server/lib/ActiveDoc.ts
View File

@@ -398,7 +398,7 @@ export class ActiveDoc extends EventEmitter {
await this._actionHistory.initialize();
this._granularAccess = new GranularAccess(this.docData, (query) => {
return this.fetchQuery(makeExceptionalDocSession('system'), query, true)
return this.fetchQuery(makeExceptionalDocSession('system'), query, true);
});
await this._granularAccess.update();
this._sharing = new Sharing(this, this._actionHistory);
@@ -581,7 +581,7 @@ export class ActiveDoc extends EventEmitter {
// If user does not have rights to access what this query is asking for, fail.
const tableAccess = this._granularAccess.getTableAccess(docSession, query.tableId);
if (!(tableAccess.permission & Permissions.VIEW)) {
if (!(tableAccess.permission & Permissions.VIEW)) { // tslint:disable-line:no-bitwise
throw new Error('not authorized to read table');
}
@@ -1042,6 +1042,7 @@ export class ActiveDoc extends EventEmitter {
* isModification: true if document was changed by one or more actions.
* }
*/
@ActiveDoc.keepDocOpen
protected async _applyUserActions(docSession: OptDocSession, actions: UserAction[],
options: ApplyUAOptions = {}): Promise<ApplyUAResult> {
@@ -1125,6 +1126,10 @@ export class ActiveDoc extends EventEmitter {
* collaborators.
*/
private async _migrate(docSession: OptDocSession): Promise<void> {
// TODO fetchAllTables() creates more memory pressure than usual since full data is present in
// memory at once both in node and in Python. This is liable to cause crashes. This doesn't
// even skip onDemand tables. We should try to migrate using metadata only. Data migrations
// would need to be done table-by-table, and differently still for on-demand tables.
const allTables = await this.docStorage.fetchAllTables();
const docActions: DocAction[] = await this._dataEngine.pyCall('create_migrations', allTables);
this.logInfo(docSession, "_migrate: applying %d migration actions", docActions.length);

77
app/server/lib/DocStorage.ts
View File

@@ -34,6 +34,8 @@ const debuglog = util.debuglog('db');
const maxSQLiteVariables = 500; // Actually could be 999, so this is playing it safe.
const PENDING_VALUE = [gristTypes.GristObjCode.Pending];
export class DocStorage implements ISQLiteDB {
// ======================================================================
@@ -332,6 +334,40 @@ export class DocStorage implements ISQLiteDB {
}
}
},
async function(db: SQLiteDB): Promise<void> {
// Storage version 7. Migration to store formulas in SQLite.
// Here, we only create empty columns for each formula column in the document. We let
// ActiveDoc, when it calculates formulas on open, detect that this migration just
// happened, and save the calculated results.
const colRows: ResultRow[] = await db.all('SELECT t.tableId, c.colId, c.type ' +
'FROM _grist_Tables_column c JOIN _grist_Tables t ON c.parentId=t.id WHERE c.isFormula');
// Go table by table.
const tableColRows = groupBy(colRows, 'tableId');
for (const tableId of Object.keys(tableColRows)) {
// There should be no columns conflicting with formula columns, but we check and skip
// them if there are.
const infoRows = await db.all(`PRAGMA table_info(${quoteIdent(tableId)})`);
const presentCols = new Set([...infoRows.map(row => row.name)]);
const newCols = tableColRows[tableId].filter(c => !presentCols.has(c.colId));
// Create all new columns.
for (const {colId, type} of newCols) {
await db.exec(`ALTER TABLE ${quoteIdent(tableId)} ` +
`ADD COLUMN ${DocStorage._columnDef(colId, type)}`);
}
// Fill them in with PENDING_VALUE. This way, on first load and Calculate, they would go
// from "Loading..." to their proper value. After the migration, they should never have
// PENDING_VALUE again.
const colListSql = newCols.map(c => `${quoteIdent(c.colId)}=?`).join(', ');
const types = newCols.map(c => c.type);
const sqlParams = DocStorage._encodeColumnsToRows(types, newCols.map(c => [PENDING_VALUE]));
await db.run(`UPDATE ${quoteIdent(tableId)} SET ${colListSql}`, sqlParams[0]);
}
},
]
};
@@ -758,9 +794,7 @@ export class DocStorage implements ISQLiteDB {
public _process_AddTable(tableId: string, columns: any[]): Promise<void> {
const colSpecSql =
DocStorage._prefixJoin(', ',
columns.filter(c =>
!c.isFormula).map(c =>
DocStorage._columnDef(c.id, c.type)));
columns.map(c => DocStorage._columnDef(c.id, c.type)));
// Every table needs an "id" column, and it should be an "integer primary key" type so that it
// serves as the alias for the SQLite built-in "rowid" column. See
@@ -923,10 +957,6 @@ export class DocStorage implements ISQLiteDB {
* @returns {Promise} - A promise for the SQL execution.
*/
public async _process_AddColumn(tableId: string, colId: string, colInfo: any): Promise<void> {
// No need to ALTER TABLE for formula columns
if (colInfo.isFormula) {
return;
}
await this.exec(
`ALTER TABLE ${quoteIdent(tableId)} ADD COLUMN ${DocStorage._columnDef(colId, colInfo.type)}`);
}
@@ -943,14 +973,8 @@ export class DocStorage implements ISQLiteDB {
if (fromColId === 'id' || fromColId === 'manualSort' || tableId.startsWith('_grist')) {
throw new Error('Cannot rename internal Grist column');
}
try {
await this.exec(
`ALTER TABLE ${quoteIdent(tableId)} RENAME COLUMN ${quoteIdent(fromColId)} TO ${quoteIdent(toColId)}`);
} catch (error) {
if (!String(error).match(/SQLITE_ERROR: no such column/)) { throw error; }
// Accept no-such-column, because this may be a formula column.
// TODO: tighten constraint by getting access to grist schema info and isFormula flag.
}
await this.exec(
`ALTER TABLE ${quoteIdent(tableId)} RENAME COLUMN ${quoteIdent(fromColId)} TO ${quoteIdent(toColId)}`);
}
/**
@@ -968,20 +992,7 @@ export class DocStorage implements ISQLiteDB {
log.error("ModifyColumn action called without params.");
return;
}
if (colInfo.isFormula) {
return this._process_RemoveColumn(tableId, colId);
} else {
const colExists = await this._colExistsInDB(tableId, colId);
const toNonFormula = colInfo.hasOwnProperty('isFormula') && !colInfo.isFormula;
if (!colExists && toNonFormula) {
// It's important to have the original type here, when colInfo does not include it.
if (!colInfo.type) { colInfo.type = this._getGristType(tableId, colId); }
return this._process_AddColumn(tableId, colId, colInfo);
} else {
return this._alterColumn(tableId, colId, colId, colInfo.type);
}
}
return this._alterColumn(tableId, colId, colId, colInfo.type);
}
@@ -1301,14 +1312,6 @@ export class DocStorage implements ISQLiteDB {
}
}
/**
* Returns a promise for a boolean for whether the given column exists in the database.
*/
private _colExistsInDB(tableId: string, colId: string): Promise<boolean> {
return this.all(`PRAGMA table_info(${quoteIdent(tableId)})`)
.then(infoRows => infoRows.some(row => (row.name === colId)));
}
private _getGristType(tableId: string, colId: string): string {
return (this._docSchema[tableId] && this._docSchema[tableId][colId]) || 'Any';
}

23
app/server/lib/ExpandedQuery.ts
View File

@@ -36,9 +36,9 @@ export interface ExpandedQuery extends Query {
* The referenced column itself cannot (yet) be a formula.
* Filtered columns cannot (yet) be a formula.
*
* If formulas is not set, we simply mark formula columns as pending.
* If onDemandFormulas is set, ignore stored formula columns, and compute them using SQL.
*/
export function expandQuery(iquery: Query, docData: DocData, formulas: boolean = true): ExpandedQuery {
export function expandQuery(iquery: Query, docData: DocData, onDemandFormulas: boolean = true): ExpandedQuery {
const query: ExpandedQuery = {
tableId: iquery.tableId,
filters: iquery.filters,
@@ -63,11 +63,12 @@ export function expandQuery(iquery: Query, docData: DocData, formulas: boolean =
const joins = new Set<string>();
const selects = new Set<string>();
// Select all data columns
selects.add(`${quoteIdent(query.tableId)}.*`);
// Iterate through all formulas, adding joins and selects as we go.
if (formulas) {
if (onDemandFormulas) {
selects.add(`${quoteIdent(query.tableId)}.id`);
for (const column of dataColumns) {
selects.add(`${quoteIdent(query.tableId)}.${quoteIdent(column.colId as string)}`);
}
const formulaColumns = columns.filterRecords({parentId: tableRef, isFormula: true});
for (const column of formulaColumns) {
const formula = parseFormula(column.formula as string);
@@ -118,14 +119,8 @@ export function expandQuery(iquery: Query, docData: DocData, formulas: boolean =
}
}
} else {
const formulaColumns = columns.filterRecords({parentId: tableRef, isFormula: true});
for (const column of formulaColumns) {
if (!column.formula) { continue; } // Columns like this won't get calculated, so skip.
const colId = column.colId as string;
if (!query.constants) { query.constants = {}; }
query.constants[colId] = ['P'];
selects.add(`0 as ${quoteIdent(colId)}`);
}
// Select all data and formula columns.
selects.add(`${quoteIdent(query.tableId)}.*`);
}
// Copy decisions to the query object, and return.

14
app/server/lib/Sharing.ts
View File

@@ -213,9 +213,10 @@ export class Sharing {
// action that initializes them when the document is opened cold
// (without cached ActiveDoc) - otherwise we'll end up with spam
// log entries for each time the document is opened cold.
const trivial = (userActions.length === 1 &&
userActions[0][0] === 'Calculate' &&
sandboxActionBundle.stored.length === 0);
const isCalculate = (userActions.length === 1 &&
userActions[0][0] === 'Calculate');
const trivial = isCalculate && sandboxActionBundle.stored.length === 0;
const actionNum = trivial ? 0 :
(branch === Branch.Shared ? this._actionHistory.getNextHubActionNum() :
@@ -256,7 +257,9 @@ export class Sharing {
// be shared. Once sharing is enabled, we would share a snapshot at that time.
await this._actionHistory.recordNextShared(localActionBundle);
}
if (client && client.clientId) {
// Check isCalculate because that's not an action we should allow undo/redo for (it's not
// considered as performed by a particular client).
if (client && client.clientId && !isCalculate) {
this._actionHistory.setActionClientId(localActionBundle.actionHash!, client.clientId);
}
});
@@ -274,6 +277,9 @@ export class Sharing {
client,
retValues: sandboxActionBundle.retValues,
summarize: true,
// Mark the on-open Calculate action as internal. In future, synchronizing fields to today's
// date and other changes from external values may count as internal.
internal: isCalculate,
});
await this._activeDoc.broadcastDocUpdate(client || null, 'docUserAction', {
actionGroup,