DBSelect.orWhere #3

Merged
merged 6 commits into from Aug 14, 2012

Conversation

Projects
None yet
2 participants
Collaborator

kalifg commented Aug 2, 2012

specifies whether to use OR instead of AND for the current clause.

Added the function DBSelect.orWhere to give it parity with ZendDB.

Modified the function DBSelect.assemble to take advantage of this
feature.

@kalifg kalifg The function DBSelect.where now has an optional third parameter that
specifies whether to use OR instead of AND for the current clause.

Added the function DBSelect.orWhere to give it parity with ZendDB.

Modified the function DBSelect.assemble to take advantage of this
feature.
ec670eb
Owner

DrBenton commented Aug 3, 2012

Hey, this is a simple but efficient feature ! :-)
Before merging, could you add some unit tests about this new feature in "test/db-select.js" ?

Thanks a lot !

Owner

DrBenton commented Aug 3, 2012

Thank you for the unit test !

Shouldn't we add parentheses around each condition, like Zend_Db does ?

Zend_Db_Select automatically puts parentheses around each expression you specify using the where() or orWhere() > methods. This helps to ensure that Boolean operator precedence does not cause unexpected results.
http://framework.zend.com/manual/en/zend.db.select.html

This would prevent problems with queries like this one (from the same Zend_Db documentation page) :

$select = $db->select()
             ->from('products',
                    array('product_id', 'product_name', 'price'))
             ->where("price < $minimumPrice OR price > $maximumPrice")
             ->where('product_name = ?', $prod);
Collaborator

kalifg commented Aug 3, 2012

I was just working on that! :)

The problem with automatically grouping the OR clauses is what if you want the normal precedence rules?

ie, you want a AND b OR c AND d to be (a AND b) OR (c AND d), not a AND (b OR c) AND d

I was also thinking that to avoid the hack of putting ORs inside the string , I could code something so that we could do something like this:

dbSelect.from('products', ['product_id', 'product_name', 'price'])
    .whereGroup()
    .where('price < ?', minimumPrice)
    .orWhere('price > ?', maximumPrice)
    .whereGroupClose()
    .where('product_name = ?', prod)

We could keep a grouping count to make sure that no invalid sql strings are generated upon assemble. This should allow arbitrary grouping structures. What do you think?

@kalifg kalifg Added new functions DBSelect.whereGroup(groups) and
DBSelect.whereGroupClose(groups).

These function will allow the creation of arbitrary parenthetical
expressions in the WHERE clause.  Each has a parameter specifying how
many groups to open or close.  It's smart enough to not close groups
that aren't open and to automatically close open groups even if you
forget to.

As an example, this:

dbWrapper.getSelect()
          .from('user')
          .whereGroupClose() // Erroneous whereGroupClose to be handled
          .whereGroup(3)     // Multiple group start
          .where('enabled=1')
          .where( 'id=?', 10 )
          .whereGroupClose()
          .where( 'first_name=?', 'Dr.')
          .whereGroup()
          .where( 'last_name LIKE ?', '%Benton%' )
          .orWhere( 'nickname=?', 'dude' )
          .whereGroupClose(2);
          // Automatic closing of leftover groups

Generates this:

"SELECT user.* FROM user WHERE (((enabled=1 AND id=10) AND
first_name='Dr.' AND (last_name LIKE '%Benton%' OR nickname='dude')))"
8bab6d2
Collaborator

kalifg commented Aug 3, 2012

I went ahead pushed the new functionality, with unit tests. Please let me know if there is something you'd like changed. This is the best way I can think of to offer full functionality, and it doesn't change the behavior of any existing code to boot.

Owner

DrBenton commented Aug 5, 2012

Hi !
Well, it seems simple and efficient!
I would have a last request : as you are a native english speaker, would you mind adding some documentation about these new whereGroup() / whereGroupClose() statements in the README? You would do that far better than me, with my poor english level :-)
You can also add your name in the package.json "authors" field, if you want to.

Collaborator

kalifg commented Aug 5, 2012

Certainly, I'd be happy to! I had no idea you weren't a native english speaker, you do very well.

I will let you know when I have that finished.

@kalifg kalifg Added documentation about new where features.
Added name to package.json under authors section.
edd925b
Owner

DrBenton commented Aug 9, 2012

Thank you very much for this new functionnality!
I will just add automatic parentheses around all WHERE clauses, to allow a simple where("price < ? OR price > ?") for simple SQL queries, then I will merge your Pull Request.

Collaborator

kalifg commented Aug 9, 2012

Good idea! I get what you are saying about that now.

Collaborator

kalifg commented Aug 13, 2012

Do you need anything else from me before merging this?

Owner

DrBenton commented Aug 14, 2012

No no, it's ok. I just wanted to add the automatic parentheses around all WHERE clauses, but I don't have enough time these days :-)
So... this is merge time!

Thank you very much for this new feature!

@DrBenton DrBenton added a commit that referenced this pull request Aug 14, 2012

@DrBenton DrBenton Merge pull request #3 from kalifg/orWhere
DBSelect.orWhere
a4068c0

@DrBenton DrBenton merged commit a4068c0 into DrBenton:master Aug 14, 2012

Collaborator

kalifg commented Aug 14, 2012

Oh, I'm sorry, I forgot you had said that. I would be happy to add that for you!

Michael

On Aug 14, 2012, at 3:10 AM, Olivier Philippon notifications@github.com wrote:

No no, it's ok. I just wanted to add the automatic parentheses around all WHERE clauses, but I don't have enought time these days :-)
So... this is merge time!

Thank you very much for this new feature!


Reply to this email directly or view it on GitHub.

Owner

DrBenton commented Aug 14, 2012

Well, I thought I would do it ; but if you want to, it would be great ! :-)

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