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.

Sign up for GitHub

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

Jump to bottom

Remove queued query #2480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+25 −0
Open

Remove queued query #2480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/protocol/Protocol.js
View file
Open in desktop
Expand Up @@ -336,6 +336,14 @@ Protocol.prototype._dequeue = function(sequence) {
return;
}

if (sequence !== this._queue[0]) {
var idx = this._queue.indexOf(sequence);
if (idx > 0) {
Copy link

Choose a reason for hiding this comment

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

>= ?

Copy link
Author

Choose a reason for hiding this comment

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

idx will never be 0 as sequence !== this._queue[0]. The sequence in this._queue[0] is the one that is currently being executed, isn't it?

Or you mean to change it to idx >= 1?

Copy link

Choose a reason for hiding this comment

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

My bad, missed previous line.

Btw maybe use separate method for removing sequence from queue? Its seems current method executes next sequence, but im not sure.

Copy link
Author

Choose a reason for hiding this comment

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

@maximelkin I don't call Protocol._dequeue(sequence) directly. It is called from handler of Sequence.emit('end') emited from Query.end() called in my Query.dequeue(). I wanted to use the native query ending mechanism, so the query will call its _callback (with nothing) after being cancelled.

But yes, if we think it would be better, we could short circuit the native query ending mechanism, and just remove the query from the queue without calling Query.end() and emitting Sequence.emit('end'). But then the query _callback would never be called. I'm not sure if it would be good or not.

this._queue.splice(idx, 1);
}
return;
}

this._queue.shift();

var sequence = this._queue[0];
Expand Down
17 changes: 17 additions & 0 deletions lib/protocol/sequences/Query.js
View file
Open in desktop
Expand Up @@ -24,12 +24,29 @@ function Query(options, callback) {
this._fields = [];
this._index = 0;
this._loadError = null;
this._started = false;
}

Query.prototype.start = function() {
this._started = true;
this.emit('packet', new Packets.ComQueryPacket(this.sql));
};

Query.prototype.started = function() {
return this._started;
};

Query.prototype.ended = function() {
return this._ended;
};

Query.prototype.dequeue = function() {
if (this._started) {
throw new Error('Query already started.');
}
this.end();
Copy link
Author

Choose a reason for hiding this comment

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

I'm not entirelry sure calling this.end() here won't mess something. I've tracked the code, it seems to behave as expected: it calls this._callback with no results, emits end which calls Protocol._dequeue(this). But I'm not sure I covered all cases that may happen.

};

Query.prototype.determinePacket = function determinePacket(byte, parser) {
var resultSet = this._resultSet;

Expand Down