From 18f76622095257d9a6912aeca3f6e2f65e73ac1c Mon Sep 17 00:00:00 2001 From: Anthony Dresser Date: Thu, 10 Jan 2019 13:44:14 -0800 Subject: [PATCH] Duplicate Result sets (#3620) * remove debouncing and echoing to fix rendering bug * fix access of internal member * fix issue with using splice rather than slice * fix compile issues --- src/sql/parts/query/editor/messagePanel.ts | 1 + .../query/execution/queryModelService.ts | 6 +- src/sql/parts/query/execution/queryRunner.ts | 76 ++++++++----------- 3 files changed, 33 insertions(+), 50 deletions(-) diff --git a/src/sql/parts/query/editor/messagePanel.ts b/src/sql/parts/query/editor/messagePanel.ts index 9a375ca4ac..5ea53b1fc9 100644 --- a/src/sql/parts/query/editor/messagePanel.ts +++ b/src/sql/parts/query/editor/messagePanel.ts @@ -144,6 +144,7 @@ export class MessagePanel extends ViewletPanel { this.reset(); this.queryRunnerDisposables.push(runner.onQueryStart(() => this.reset())); this.queryRunnerDisposables.push(runner.onMessage(e => this.onMessage(e))); + this.onMessage(runner.messages); } private onMessage(message: IResultMessage | IResultMessage[]) { diff --git a/src/sql/parts/query/execution/queryModelService.ts b/src/sql/parts/query/execution/queryModelService.ts index 84f22ac5c4..7a67e6fd86 100644 --- a/src/sql/parts/query/execution/queryModelService.ts +++ b/src/sql/parts/query/execution/queryModelService.ts @@ -391,10 +391,8 @@ export class QueryModelService implements IQueryModelService { queryRunner.addListener(QREvents.RESULT_SET, resultSet => { this._fireQueryEvent(ownerUri, resultSetEventType, resultSet); }); - queryRunner.onResultSetUpdate(resultSetSummaries => { - resultSetSummaries.forEach(resultSet => { - this._fireQueryEvent(ownerUri, resultSetEventType, resultSet); - }); + queryRunner.onResultSetUpdate(resultSetSummary => { + this._fireQueryEvent(ownerUri, resultSetEventType, resultSetSummary); }); queryRunner.addListener(QREvents.BATCH_START, batch => { let link = undefined; diff --git a/src/sql/parts/query/execution/queryRunner.ts b/src/sql/parts/query/execution/queryRunner.ts index 8dd792116d..1083a48df0 100644 --- a/src/sql/parts/query/execution/queryRunner.ts +++ b/src/sql/parts/query/execution/queryRunner.ts @@ -13,6 +13,7 @@ import { IQueryManagementService } from 'sql/parts/query/common/queryManagement' import * as Utils from 'sql/parts/connection/common/utils'; import { SaveFormat } from 'sql/parts/grid/common/interfaces'; import { echo, debounceEvent } from 'sql/base/common/event'; +import { Deferred } from 'sql/base/common/promise'; import Severity from 'vs/base/common/severity'; import { IWorkspaceConfigurationService } from 'vs/workbench/services/configuration/common/configuration'; @@ -26,7 +27,6 @@ import { Emitter, Event } from 'vs/base/common/event'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ResultSerializer } from 'sql/parts/query/common/resultSerializer'; import { TPromise } from 'vs/base/common/winjs.base'; -import { Deferred } from 'sql/base/common/promise'; export interface IEditSessionReadyEvent { ownerUri: string; @@ -69,6 +69,7 @@ export default class QueryRunner extends Disposable { private _isExecuting: boolean = false; private _hasCompleted: boolean = false; private _batchSets: sqlops.BatchSummary[] = []; + private _messages: sqlops.IResultMessage[] = []; private _eventEmitter = new EventEmitter(); private _isQueryPlan: boolean; @@ -77,40 +78,13 @@ export default class QueryRunner extends Disposable { public get planXml(): Thenable { return this._planXml.promise; } private _onMessage = this._register(new Emitter()); - private _debouncedMessage = debounceEvent(this._onMessage.event, (l, e) => { - // on first run - if (types.isUndefinedOrNull(l)) { - return [e]; - } else { - return l.concat(e); - } - }); - private _echoedMessages = echo(this._debouncedMessage.event); - public readonly onMessage = this._echoedMessages.event; + public readonly onMessage = this._onMessage.event; private _onResultSet = this._register(new Emitter()); - private _debouncedResultSet = debounceEvent(this._onResultSet.event, (l, e) => { - // on first run - if (types.isUndefinedOrNull(l)) { - return [e]; - } else { - return l.concat(e); - } - }); - // private _echoedResultSet = echo(this._debouncedResultSet.event); - public readonly onResultSet = this._debouncedResultSet.event; + public readonly onResultSet = this._onResultSet.event; private _onResultSetUpdate = this._register(new Emitter()); - private _debouncedResultSetUpdate = debounceEvent(this._onResultSetUpdate.event, (l, e) => { - // on first run - if (types.isUndefinedOrNull(l)) { - return [e]; - } else { - return l.concat(e); - } - }); - // private _echoedResultSetUpdate = echo(this._debouncedResultSetUpdate.event); - public readonly onResultSetUpdate = this._debouncedResultSetUpdate.event; + public readonly onResultSetUpdate = this._onResultSetUpdate.event; private _onQueryStart = this._register(new Emitter()); public readonly onQueryStart: Event = this._onQueryStart.event; @@ -153,8 +127,18 @@ export default class QueryRunner extends Disposable { return this._hasCompleted; } - get batchSets(): sqlops.BatchSummary[] { - return this._batchSets; + /** + * For public use only, for private use, directly access the member + */ + public get batchSets(): sqlops.BatchSummary[] { + return this._batchSets.slice(0); + } + + /** + * For public use only, for private use, directly access the member + */ + public get messages(): sqlops.IResultMessage[] { + return this._messages.slice(0); } // PUBLIC METHODS ====================================================== @@ -202,10 +186,6 @@ export default class QueryRunner extends Disposable { if (this.isExecuting) { return TPromise.as(undefined); } - this._echoedMessages.clear(); - // this._echoedResultSet.clear(); - this._debouncedMessage.clear(); - // this._debouncedResultSet.clear(); this._planXml = new Deferred(); this._batchSets = []; this._hasCompleted = false; @@ -274,7 +254,7 @@ export default class QueryRunner extends Disposable { this._hasCompleted = true; this._batchSets = result.batchSummaries ? result.batchSummaries : []; - this.batchSets.map(batch => { + this._batchSets.map(batch => { if (batch.selection) { batch.selection.startLine = batch.selection.startLine + this._resultLineOffset; batch.selection.endLine = batch.selection.endLine + this._resultLineOffset; @@ -291,6 +271,7 @@ export default class QueryRunner extends Disposable { isError: false, time: undefined }; + this._messages.push(message); this._onQueryEnd.fire(timeStamp); this._onMessage.fire(message); @@ -312,7 +293,7 @@ export default class QueryRunner extends Disposable { batch.resultSetSummaries = []; // Store the batch - this.batchSets[batch.id] = batch; + this._batchSets[batch.id] = batch; let message = { // account for index by 1 @@ -321,6 +302,7 @@ export default class QueryRunner extends Disposable { selection: batch.selection, isError: false }; + this._messages.push(message); this._eventEmitter.emit(EventType.BATCH_START, batch); this._onMessage.fire(message); this._onBatchStart.fire(batch); @@ -333,7 +315,7 @@ export default class QueryRunner extends Disposable { let batch: sqlops.BatchSummary = result.batchSummary; // Store the batch again to get the rest of the data - this.batchSets[batch.id] = batch; + this._batchSets[batch.id] = batch; let executionTime = (Utils.parseTimeString(batch.executionElapsed) || 0); this._totalElapsedMilliseconds += executionTime; if (executionTime > 0) { @@ -355,8 +337,8 @@ export default class QueryRunner extends Disposable { if (!resultSet.batchId) { // Missing the batchId. In this case, default to always using the first batch in the list // or create one in the case the DMP extension didn't obey the contract perfectly - if (this.batchSets.length > 0) { - batchSet = this.batchSets[0]; + if (this._batchSets.length > 0) { + batchSet = this._batchSets[0]; } else { batchSet = { id: 0, @@ -364,10 +346,10 @@ export default class QueryRunner extends Disposable { hasError: false, resultSetSummaries: [] }; - this.batchSets[0] = batchSet; + this._batchSets[0] = batchSet; } } else { - batchSet = this.batchSets[resultSet.batchId]; + batchSet = this._batchSets[resultSet.batchId]; } // handle getting queryPlanxml if we need too if (this.isQueryPlan) { @@ -392,7 +374,7 @@ export default class QueryRunner extends Disposable { if (result && result.resultSetSummary) { let resultSet = result.resultSetSummary; let batchSet: sqlops.BatchSummary; - batchSet = this.batchSets[resultSet.batchId]; + batchSet = this._batchSets[resultSet.batchId]; // handle getting queryPlanxml if we need too if (this.isQueryPlan) { // check if this result has show plan, this needs work, it won't work for any other provider @@ -415,6 +397,7 @@ export default class QueryRunner extends Disposable { public handleMessage(obj: sqlops.QueryExecuteMessageParams): void { let message = obj.message; message.time = new Date(message.time).toLocaleTimeString(); + this._messages.push(message); // Send the message to the results pane this._eventEmitter.emit(EventType.MESSAGE, message); @@ -636,7 +619,7 @@ export default class QueryRunner extends Disposable { private getColumnHeaders(batchId: number, resultId: number, range: Slick.Range): string[] { let headers: string[] = undefined; - let batchSummary: sqlops.BatchSummary = this.batchSets[batchId]; + let batchSummary: sqlops.BatchSummary = this._batchSets[batchId]; if (batchSummary !== undefined) { let resultSetSummary = batchSummary.resultSetSummaries[resultId]; headers = resultSetSummary.columnInfo.slice(range.fromCell, range.toCell + 1).map((info, i) => { @@ -669,6 +652,7 @@ export default class QueryRunner extends Disposable { time: undefined, isError: false }; + this._messages.push(message); // Send the message to the results pane this._onMessage.fire(message); }