Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #1391 (closed enhancement: duplicate)

Opened 3 years ago

Last modified 3 years ago

[XPATCH] Switch = null and <> null to is null and is not null

Reported by: wishdev@gmail.com Assigned to: David
Priority: low Milestone: 1.1
Component: ActiveRecord Version: 0.12.1
Severity: normal Keywords: null sql query
Cc:

Description

The following patches should address the areas where = null is currently used in the code. They have been switched to the appropriate is/is not null statements. Some of the code is a little "clunky" and verbose - please feel free to optimize at your pleasure. I like it a little more verbose when first making changes like this.

Attachments

association_proxy.patch (0.6 kB) - added by wishdev@gmail.com on 06/02/05 20:28:04.
base.patch (2.6 kB) - added by wishdev@gmail.com on 06/02/05 20:28:28.
has_and_belongs_to_many_association.patch (1.3 kB) - added by wishdev@gmail.com on 06/02/05 20:28:57.
has_many_association.patch (2.1 kB) - added by wishdev@gmail.com on 06/02/05 20:29:19.
has_one_association.patch (0.7 kB) - added by wishdev@gmail.com on 06/02/05 20:29:41.
validations.patch (1.7 kB) - added by wishdev@gmail.com on 06/02/05 20:30:00.
association_proxy_2.patch (0.6 kB) - added by wishdev@gmail.com on 06/02/05 20:43:00.
Small typo fixed
has_many_association_2.patch (2.1 kB) - added by wishdev@gmail.com on 06/02/05 20:44:04.
Another little typo
single_file.diff (8.5 kB) - added by wishdev@gmail.com on 06/03/05 18:23:47.
As request - unified single patch!

Change History

06/02/05 20:28:04 changed by wishdev@gmail.com

  • attachment association_proxy.patch added.

06/02/05 20:28:28 changed by wishdev@gmail.com

  • attachment base.patch added.

06/02/05 20:28:57 changed by wishdev@gmail.com

  • attachment has_and_belongs_to_many_association.patch added.

06/02/05 20:29:19 changed by wishdev@gmail.com

  • attachment has_many_association.patch added.

06/02/05 20:29:41 changed by wishdev@gmail.com

  • attachment has_one_association.patch added.

06/02/05 20:30:00 changed by wishdev@gmail.com

  • attachment validations.patch added.

06/02/05 20:43:00 changed by wishdev@gmail.com

  • attachment association_proxy_2.patch added.

Small typo fixed

06/02/05 20:44:04 changed by wishdev@gmail.com

  • attachment has_many_association_2.patch added.

Another little typo

06/02/05 20:46:07 changed by wishdev@gmail.com

Sorry I didn't run the unit tests before sending up the first set of patches. Got a little quick on the draw :) SQLite test now passes without additional errors (I have 6 errors on the current CVS version with or without these modifications).

06/02/05 21:54:08 changed by bitsweat

  • keywords changed from null acritverecord to null sql query.
  • priority changed from normal to low.
  • severity changed from normal to enhancement.

Does the = and <> behavior break any of the databases or is this purely an aesthetic issue? I'm assuming the latter and changing it to an enhancement request (no worries, aesthetics are cool too ;)

Also, could you submit your changes as a single unified diff? 

cd /path/to/your/svn/checkout
svn diff > mypatch.diff

Thanks!

06/02/05 22:17:56 changed by wishdev@gmail.com

There is already a patch in the system concerning Postgresql (#1213) and possible breakage - and also my Firebird Adapter (gotta submit the patch - gotta submit the patch) will break without these changes. Firebird is totally intolerant of = null. So it's sort of an enhancement to get another adapter in the code base I guess :) I will get the unified patch up asap - didn't know which would be best for you :)

06/02/05 23:35:40 changed by nzkoz

Indeed, I definitely prefer the approach here to the one in #1213. If this is applied, we should close the other one.

06/03/05 18:23:47 changed by wishdev@gmail.com

  • attachment single_file.diff added.

As request - unified single patch!

06/25/05 11:36:34 changed by david

  • summary changed from [PATCH] Switch = null and <> null to is null and is not null to [XPATCH] Switch = null and <> null to is null and is not null.

This looks like a good idea, but the patch features some dauntingly complex if/else pairs and doesn't have any tests to back it up. It would be nice if the code could be made more clear and tests added. I wouldn't be confident applying the patch in its current state.

07/18/05 03:59:38 changed by nzkoz

  • milestone set to 1.x.

Setting all XPATCHs out to 1.x

11/16/05 07:05:11 changed by bitsweat

  • status changed from new to closed.
  • resolution set to duplicate.

Duplicate #1874.