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

Support multi-column `whereIn` with query #1390

Merged
merged 8 commits into from Feb 21, 2018

Conversation

Projects
None yet
3 participants
@rhys-vdw
Collaborator

rhys-vdw commented May 8, 2016

Add support for supplying a subquery for a whereIn statment on multiple columns.

Fixes #1384.

Support multi-column `whereIn` with query
Add support for supplying a subquery for a `whereIn` statment on multiple columns.

Fixes #1384.
@wubzz

This comment has been minimized.

Collaborator

wubzz commented May 8, 2016

@rhys-vdw Looks good!

Perhaps we can use this new formatter function for non-string literals in .select() / .from() as well which has been brought up before. Currently doing fancy stuff with those two functions requires some hacky workarounds with raw.

@@ -26,7 +27,7 @@ assign(Formatter.prototype, {
// a "joining" value is specified (e.g. ' and ')
parameterize(values, notSetValue) {
if (typeof values === 'function') return this.parameter(values);
values = Array.isArray(values) ? values : [values];
values = isArray(values) ? values : [values];

This comment has been minimized.

@wubzz

wubzz May 9, 2016

Collaborator

Not sure I follow on this change. const { isArray } = Array; aren't you then constructing a new Array with values here? Same as Array(null) Array(void 0) Array([1,2,3]). Would mean the ternary operation cant fail?

Or am I just missing something obvious?

This comment has been minimized.

@rhys-vdw

rhys-vdw May 10, 2016

Collaborator

It is actually no change. That translates to:

const { isArray } = Array;
// compiles to:
var isArray = Array.isArray;

See object destructuring.

@rhys-vdw

This comment has been minimized.

Collaborator

rhys-vdw commented May 14, 2016

I'm holding off merging this for now. I want to test performance of Formatter#values and also use it function in other relevant sections.

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Feb 8, 2018

Looks like this has been already implemented https://runkit.com/embed/f2wym1fwfrn1 ?

EDIT: right.. it was for query as a parameter :)

@elhigu elhigu closed this Feb 8, 2018

@elhigu elhigu reopened this Feb 8, 2018

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Feb 8, 2018

I tried to fix these conflicts in git web editor... needs reviewing again if it still works as intended.

EDIT: yup it got broken while resolving in webui 😬 need to take checkout to laptop

elhigu added some commits Feb 8, 2018

@elhigu elhigu referenced this pull request Feb 20, 2018

Closed

Adding Prettier #2495

@wubzz

This comment has been minimized.

Collaborator

wubzz commented Feb 21, 2018

Updated the failing tests here (was due to 'default' old assertion type) and added assertions to remaining dialects.

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Feb 21, 2018

Thanks!

@elhigu

Documentation of multi-column whereIn() calls is still missing, feature looks good 👍

@elhigu

elhigu approved these changes Feb 21, 2018

@elhigu elhigu merged commit 965542d into master Feb 21, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment