Skip to content
This repository has been archived by the owner on Nov 9, 2021. It is now read-only.

quoting field names in where(), having() and expressions #224

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nlang
Copy link

@nlang nlang commented Mar 29, 2016

I added auto-quoting ability to WhereBlock.where(), HavingBlock.having(), Expression.or() and Expression.and() by providing an alternate method signature (field, operator, ...params). All existing tests pass and I added new tests to check the quoting feature. This is also related to issue #164

So for example if you have autoQuoteFieldNames = true and you call
.where("field", "=", 42) you will get WHERE field = 42
If you call .where("field = ?", 42) you will get WHERE field = 42 as before.

So its:
.where("field = ?", 42) vs. .where("field", "=", 42)
.having("field <> ?", "val") vs. .having("field", "<>", "val")
.and("field IN ?", [4, 8, 15, 16, 23, 42]) vs. .where("field", "IN", [4, 8, 15, 16, 23, 42])
.or("field like ?", "~foo") vs. .where("field", "like", "~foo")

Recognized operators are ['=', '<', '>', '<=', '>=', '<>', '!=', 'in', 'not in', 'like', 'not like']

nlang added 4 commits March 29, 2016 22:43
…), Expression.or() and Expression.and() by providing an alternate method signature (field, operator, ...params)
…), Expression.or() and Expression.and() by providing an alternate method signature (field, operator, ...params)
@hiddentao
Copy link
Owner

Thanks for this. I'll need some time to review the code. Looks good so far.

@nlang
Copy link
Author

nlang commented Mar 31, 2016

Thanks for your reply. There are still some issues and cases that won't work well, like BETWEEN(? AND ?) for example. I will fix this in another commit, but that will take me a few days.

having (condition, ...values) {
this._condition(condition, ...values);
having (field, operator, ...values) {
let validOperators = ['=', '<', '>', '<=', '>=', '<>', '!=', 'in', 'not in', 'like', 'not like', 'is', 'is not'];
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like we're repeating the code which checks for a valid operator and then building a string out of it. I think this checking code should be shifted into the BaseBuilder class. Furthermore, the then-block code should be inside AbstractConditionBlock so that all subclasses automatically benefit from it. I think having() can then go back to just being a single-line function. Do you agree?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for looking into this. Yes, I'm aware of the duplication and I agree. I have some improvements planned for defining and checking the valid operators. I will submit them in another commit soon. The duplicate code will be gone then and I will move the code blocks to the appropriate base classes as you suggested.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants