From f96fd911c1d0d883587748de8560ffc36d17a17d Mon Sep 17 00:00:00 2001 From: Lucy Zhang Date: Mon, 7 Dec 2020 12:28:07 -0800 Subject: [PATCH] Notebooks: Remove result set summary from saved metadata (#13616) * remove result set summary from metadata * remove batchId and id from celloutputmetadata * remove extra line --- src/sql/azdata.proposed.d.ts | 4 ---- .../browser/cellViews/output.component.ts | 12 ---------- .../cellViews/outputArea.component.html | 2 +- .../browser/outputs/gridOutput.component.ts | 14 +++++------ .../notebook/test/browser/sqlFuture.test.ts | 9 +++---- .../services/notebook/browser/models/cell.ts | 24 +++++++++++++++---- .../browser/models/modelInterfaces.ts | 2 ++ .../notebook/browser/sql/sqlSessionManager.ts | 9 +++---- 8 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index e4ce5fbbf6..186d884aaa 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/src/sql/azdata.proposed.d.ts @@ -74,10 +74,6 @@ declare module 'azdata' { data: any; } - export interface ICellOutputMetadata { - resultSet?: ResultSetSummary; - } - export interface INotebookMetadata { connection_name?: string; } diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/output.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/output.component.ts index b8a68438dd..bf890859eb 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/output.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/output.component.ts @@ -40,8 +40,6 @@ export class OutputComponent extends CellView implements OnInit, AfterViewInit { private _initialized: boolean = false; private _activeCellId: string; private _componentInstance: IMimeComponent; - private _batchId: number | undefined; - private _id: number | undefined; public errorText: string; constructor( @@ -103,14 +101,6 @@ export class OutputComponent extends CellView implements OnInit, AfterViewInit { return this._componentInstance; } - @Input() set batchId(value: number | undefined) { - this._batchId = value; - } - - @Input() set id(value: number | undefined) { - this._id = value; - } - get trustedMode(): boolean { return this._trusted; } @@ -183,8 +173,6 @@ export class OutputComponent extends CellView implements OnInit, AfterViewInit { this._componentInstance.cellModel = this.cellModel; this._componentInstance.cellOutput = this.cellOutput; this._componentInstance.bundleOptions = options; - this._componentInstance.batchId = this._batchId; - this._componentInstance.id = this._id; this._changeref.detectChanges(); let el = componentRef.location.nativeElement; diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/outputArea.component.html b/src/sql/workbench/contrib/notebook/browser/cellViews/outputArea.component.html index 80a9d97e0f..f106cd9692 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/outputArea.component.html +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/outputArea.component.html @@ -6,7 +6,7 @@ -->
- +
diff --git a/src/sql/workbench/contrib/notebook/browser/outputs/gridOutput.component.ts b/src/sql/workbench/contrib/notebook/browser/outputs/gridOutput.component.ts index 37d816617d..00c795b719 100644 --- a/src/sql/workbench/contrib/notebook/browser/outputs/gridOutput.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/outputs/gridOutput.component.ts @@ -40,6 +40,7 @@ import { ActionsOrientation } from 'vs/base/browser/ui/actionbar/actionbar'; import { values } from 'vs/base/common/collections'; import { URI } from 'vs/base/common/uri'; import { assign } from 'vs/base/common/objects'; +import { QueryResultId } from 'sql/workbench/services/notebook/browser/models/cell'; import { equals } from 'vs/base/common/arrays'; @Component({ selector: GridOutputComponent.SELECTOR, @@ -94,16 +95,13 @@ export class GridOutputComponent extends AngularDisposable implements IMimeCompo this._cellOutput = value; } - @Input() set batchId(value: number | undefined) { - this._batchId = value; - } - - @Input() set id(value: number | undefined) { - this._id = value; - } - ngOnInit() { if (this.cellModel) { + let outputId: QueryResultId = this.cellModel.getOutputId(this._cellOutput); + if (outputId) { + this._batchId = outputId.batchId; + this._id = outputId.id; + } this._register(this.cellModel.onTableUpdated(e => { if (e.resultSet.batchId === this._batchId && e.resultSet.id === this._id) { this.updateResult(e.resultSet, e.rows); diff --git a/src/sql/workbench/contrib/notebook/test/browser/sqlFuture.test.ts b/src/sql/workbench/contrib/notebook/test/browser/sqlFuture.test.ts index 7645804029..0fef73544a 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/sqlFuture.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/sqlFuture.test.ts @@ -51,13 +51,14 @@ suite('SQL Future', function () { }, content: { output_type: 'execute_result', - metadata: { - resultSet: resultSet - }, + metadata: undefined, execution_count: this._executionCount, data: expectedData }, - metadata: undefined, + metadata: { + batchId: 0, + id: 0 + }, parent_header: undefined }; diff --git a/src/sql/workbench/services/notebook/browser/models/cell.ts b/src/sql/workbench/services/notebook/browser/models/cell.ts index 080bfdf523..e5daa93959 100644 --- a/src/sql/workbench/services/notebook/browser/models/cell.ts +++ b/src/sql/workbench/services/notebook/browser/models/cell.ts @@ -28,11 +28,15 @@ import { ICommandService } from 'vs/platform/commands/common/commands'; import { tryMatchCellMagic, extractCellMagicCommandPlusArgs } from 'sql/workbench/services/notebook/browser/utils'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { Disposable } from 'vs/base/common/lifecycle'; -import { ResultSetSummary } from 'sql/workbench/services/query/common/query'; let modelId = 0; const ads_execute_command = 'ads_execute_command'; +export interface QueryResultId { + batchId: number; + id: number; +} + export class CellModel extends Disposable implements ICellModel { public id: string; @@ -43,6 +47,7 @@ export class CellModel extends Disposable implements ICellModel { private _cellGuid: string; private _future: FutureInternal; private _outputs: nb.ICellOutput[] = []; + private _outputsIdMap: Map = new Map(); private _renderedOutputTextContent: string[] = []; private _isEditMode: boolean; private _onOutputsChanged = new Emitter(); @@ -238,6 +243,7 @@ export class CellModel extends Disposable implements ICellModel { this._cellType = type; // Regardless, get rid of outputs; this matches Jupyter behavior this._outputs = []; + this._outputsIdMap.clear(); } } @@ -515,6 +521,7 @@ export class CellModel extends Disposable implements ICellModel { try { // Need to reset outputs here (kernels do this on their own) this._outputs = []; + this._outputsIdMap.clear(); let commandExecuted = this._commandService?.executeCommand(result.commandId, result.args); // This will ensure that the run button turns into a stop button this.fireExecutionStateChanged(); @@ -608,6 +615,7 @@ export class CellModel extends Disposable implements ICellModel { public clearOutputs(): void { this._outputs = []; + this._outputsIdMap.clear(); this.fireOutputsChanged(); this.executionCount = undefined; @@ -634,6 +642,10 @@ export class CellModel extends Disposable implements ICellModel { return this._outputs; } + public getOutputId(output: nb.ICellOutput): QueryResultId | undefined { + return this._outputsIdMap.get(output); + } + public get renderedOutputTextContent(): string[] { return this._renderedOutputTextContent; } @@ -662,17 +674,19 @@ export class CellModel extends Disposable implements ICellModel { // Check if the table already exists for (let i = 0; i < this._outputs.length; i++) { if (this._outputs[i].output_type === 'execute_result') { - let resultSet: ResultSetSummary = this._outputs[i].metadata.resultSet; - let newResultSet: ResultSetSummary = output.metadata.resultSet; - if (resultSet.batchId === newResultSet.batchId && resultSet.id === newResultSet.id) { + let currentOutputId: QueryResultId = this._outputsIdMap.get(this._outputs[i]); + if (currentOutputId.batchId === (msg.metadata).batchId + && currentOutputId.id === (msg.metadata).id) { // If it does, update output with data resource and html table (this._outputs[i]).data = (output).data; - this._outputs[i].metadata = output.metadata; added = true; break; } } } + if (!added) { + this._outputsIdMap.set(output, { batchId: (msg.metadata).batchId, id: (msg.metadata).id }); + } break; case 'execute_result_update': let update = msg.content as nb.IExecuteResultUpdate; diff --git a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts index 2867e178d9..1bb0e1e464 100644 --- a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts +++ b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts @@ -22,6 +22,7 @@ import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/no import { IModelContentChangedEvent } from 'vs/editor/common/model/textModelEvents'; import type { FutureInternal } from 'sql/workbench/services/notebook/browser/interfaces'; import { ICellValue, ResultSetSummary } from 'sql/workbench/services/query/common/query'; +import { QueryResultId } from 'sql/workbench/services/notebook/browser/models/cell'; export interface ICellRange { readonly start: number; @@ -469,6 +470,7 @@ export interface ICellModel { executionCount: number | undefined; readonly future: FutureInternal; readonly outputs: ReadonlyArray; + getOutputId(output: nb.ICellOutput): QueryResultId | undefined; renderedOutputTextContent?: string[]; readonly onOutputsChanged: Event; readonly onTableUpdated: Event; diff --git a/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts b/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts index e2a2fe9afd..4ebc2884d1 100644 --- a/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts +++ b/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts @@ -597,13 +597,14 @@ export class SQLFuture extends Disposable implements FutureInternal { }, content: { output_type: 'execute_result', - metadata: { - resultSet: resultSet - }, + metadata: undefined, execution_count: this._executionCount, data: data }, - metadata: undefined, + metadata: { + batchId: resultSet.batchId, + id: resultSet.id + }, parent_header: undefined }; this.ioHandler.handle(msg);