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.

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

Generating the where clause for the in operator becomes inefficient with larger amount of elements #4596

Closed
haohui opened this Issue Oct 1, 2015 · 8 comments

Comments

3 participants
@haohui

haohui commented Oct 1, 2015

I noticed around 1 second of latency when generating the SQL statement with an $in operator which contains around ~180 elements.

The codes are the following:

var model = sequelize.define('ProblemState', {
  uid_pid: {type: 'BINARY(8)', primaryKey: true},
  state: Sequelize.INTEGER(6)
}

keys = [new Buffer(),...] // around 180 elements
var results = model.findAll({where: {uid_pid: {$in: keys}}, limit: 10});

It takes around 1 second to generate the sql statement. The latency does not appear if I direct run the generated query with the same set of keys via sequlize.query().

@haohui

This comment has been minimized.

haohui commented Oct 1, 2015

The latency drops to around ~100 msec if I generates an equivalent query using the $or operator (i.e., uid_pid = foo or uid_pid = bar ...).

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Oct 1, 2015

I wonder if it's related to the use of Buffer.

@janmeier

This comment has been minimized.

Member

janmeier commented Oct 1, 2015

I've narrowed it down to mapOptionFieldNames, namely this piece of code:

sequelize/lib/utils.js

Lines 133 to 139 in 7487701

if (Array.isArray(attributes[attribute])) {
attributes[attribute] = attributes[attribute].map(function (where) {
return Utils.mapOptionFieldNames({
where: where
}, Model).where;
});
}

Which calls mapOptionFieldNames recursively for each buffer.

This is to support stuff like { $or: [ { foo : 'bar' }, { baz: 'zab' }]}, where field names should be mapped properly.

I haven't been able to detect any difference between using or and in. Not really sure there's anything we can do. We might skip mapping if the key is $in but it feels like a hack to do stuff like that in utils - it shoudn't care about the keys

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Oct 2, 2015

@janmeier It's looping once for the array and then recursively calling for each buffer? That sounds like a bug, no reason to attempt to map a binary value.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Oct 2, 2015

@janmeier Couldn't we simply tell it not to do a recursive call if a primitive value here:

return Utils.mapOptionFieldNames({
?

@janmeier

This comment has been minimized.

Member

janmeier commented Oct 2, 2015

My guess what that it was just looping over each element but you're right, that wouldn't explain why this only happens to buffers.

That would probably work yeah :)

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Oct 2, 2015

Although the recursive call should be a no-op since the passed value has no keys, so not sure why this is a performance hit for buffers, do they have magic getters that incur a performance penalty that we don't know off? ;p

@janmeier

This comment has been minimized.

Member

janmeier commented Oct 4, 2015

It's caused by this line, which sends { where: Buffer } to as recursive mapOptionFieldNames call - Which then loops through the buffer because this loop is missing an isPlainObject call. Fix coming up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment