Skip to content

Remove empty key value pairs when using advanced search hash - #1653

Closed
gdott9 wants to merge 1 commit into
activerecord-hackery:mainfrom
gdott9:remove_empty_key_value_pairs
Closed

gdott9 wants to merge 1 commit into
activerecord-hackery:mainfrom
gdott9:remove_empty_key_value_pairs

Conversation

@gdott9

@gdott9 gdott9 commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

Description

When a condition in the advanced search hash has an empty value, the condition is not removed so the "LEFT OUTER JOIN" is added to the SQL query without a "WHERE" clause.

Problem

The behavior is not the same when using a simple condition hash (example: {children_name_eq: ''}) versus using an advanced search hash (example: {c: {'0' => {a: ['children_name'], p: 'eq', v: ['']}}}).

The first example results in the following query :

SELECT "people".* FROM "people" ORDER BY "people"."id" DESC

The second example results in the following query :

SELECT "people".* FROM "people" LEFT OUTER JOIN "people" "children_people" ON "children_people"."parent_id" = "people"."id" ORDER BY "people"."id" DESC

Proposed solution

I added code to remove conditions with a blank value in the same way it is done with the simple condition.
Do you see a better way to handle this ?

@scarroll32 scarroll32 closed this Sep 21, 2026
@scarroll32 scarroll32 reopened this Sep 21, 2026
scarroll32 added a commit that referenced this pull request Sep 21, 2026
A condition given through the `c:` API with an empty value was not
dropped, because the params filter in Search#initialize only looks at
top-level values and `c:` nests its values a level deeper. The condition
still built its attribute and contributed a join, so the query came out
with a LEFT OUTER JOIN and no WHERE clause to go with it — behaving
differently from the equivalent shorthand form.

Builds on @gdott9's work in #1653, with three changes.

The Hash form of `v:` was dropped unconditionally. The original guard
read `value[:value]` where `value` is the enclosing condition hash, so it
was always nil, always blank, and every Hash-form value was pruned:

  Person.ransack(c: { "0" => { a: ["name"], p: "eq",
                               v: { "0" => { value: "Ernie" } } } })
  # => SELECT "people".* FROM "people"    (condition silently gone)

A search that silently returns everything is worse than the bug being
fixed, so the value is now unwrapped from either form before testing it.

`params` is not yet indifferent-access at this point, so a string-keyed
`"c"` — the shape that arrives from an HTTP query string — was missed.
Both spellings are handled.

The blank test is factored out so the top-level filter and the `c:`
filter cannot drift apart, and so `name_in: [nil]` keeps working.

Closes #1653

Co-Authored-By: gdott9 <1830024+gdott9@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@scarroll32

Copy link
Copy Markdown
Member

Note

Comment from Claude (Claude Code), acting on behalf of @scarroll32.

Thanks @gdott9 — good catch, and the comparison between the shorthand and c: forms in your description made it easy to confirm. Picked up and finished in #1687, with you as co-author.

Three things needed changing before it could land, one of them significant:

The Hash form of v: was being dropped unconditionally. The guard reads

value[:v].all? { |_, i| value[:value].blank? && value[:value] != false }

but value there is the enclosing condition hash ({a:, p:, v:}), so value[:value] is always nil — always blank, never false. The block ignores i entirely. I ran it against your branch to be sure:

Person.ransack(c: { '0' => { a: ['name'], p: 'eq', v: { '0' => { value: 'Ernie' } } } }).result.to_sql
# => SELECT "people".* FROM "people" ORDER BY "people"."id" DESC

The condition disappears and the search returns everything. That's a worse outcome than the original bug, so #1687 unwraps the value from either the bare or { value: ... } form before testing it.

String keys. params hasn't been through with_indifferent_access yet at that point in initialize, so params.key?(:c) misses the "c" that arrives from an HTTP query string — which is how most people reach this API.

The blank test is now shared between the top-level filter and the c: filter. Your version re-implemented it, which happened to drop the !i.nil? clause added in #1657 and would have regressed name_in: [nil].

Also fixed the two Layout/SpaceAfterComma offences that were failing the build check here.

Your spec is kept, with three more covering the cases above. Closing in favour of #1687 — thanks again.

@scarroll32 scarroll32 closed this Sep 21, 2026
scarroll32 added a commit that referenced this pull request Sep 21, 2026
A condition given through the `c:` API with an empty value was not
dropped, because the params filter in Search#initialize only looks at
top-level values and `c:` nests its values a level deeper. The condition
still built its attribute and contributed a join, so the query came out
with a LEFT OUTER JOIN and no WHERE clause to go with it — behaving
differently from the equivalent shorthand form.

Builds on @gdott9's work in #1653, with three changes.

The Hash form of `v:` was dropped unconditionally. The original guard
read `value[:value]` where `value` is the enclosing condition hash, so it
was always nil, always blank, and every Hash-form value was pruned:

  Person.ransack(c: { "0" => { a: ["name"], p: "eq",
                               v: { "0" => { value: "Ernie" } } } })
  # => SELECT "people".* FROM "people"    (condition silently gone)

A search that silently returns everything is worse than the bug being
fixed, so the value is now unwrapped from either form before testing it.

`params` is not yet indifferent-access at this point, so a string-keyed
`"c"` — the shape that arrives from an HTTP query string — was missed.
Both spellings are handled.

The blank test is factored out so the top-level filter and the `c:`
filter cannot drift apart, and so `name_in: [nil]` keeps working.

Closes #1653

Co-Authored-By: gdott9 <1830024+gdott9@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
scarroll32 added a commit that referenced this pull request Sep 21, 2026
* Prune blank conditions from the low-level c: API

A condition given through the `c:` API with an empty value was not
dropped, because the params filter in Search#initialize only looks at
top-level values and `c:` nests its values a level deeper. The condition
still built its attribute and contributed a join, so the query came out
with a LEFT OUTER JOIN and no WHERE clause to go with it — behaving
differently from the equivalent shorthand form.

Builds on @gdott9's work in #1653, with three changes.

The Hash form of `v:` was dropped unconditionally. The original guard
read `value[:value]` where `value` is the enclosing condition hash, so it
was always nil, always blank, and every Hash-form value was pruned:

  Person.ransack(c: { "0" => { a: ["name"], p: "eq",
                               v: { "0" => { value: "Ernie" } } } })
  # => SELECT "people".* FROM "people"    (condition silently gone)

A search that silently returns everything is worse than the bug being
fixed, so the value is now unwrapped from either form before testing it.

`params` is not yet indifferent-access at this point, so a string-keyed
`"c"` — the shape that arrives from an HTTP query string — was missed.
Both spellings are handled.

The blank test is factored out so the top-level filter and the `c:`
filter cannot drift apart, and so `name_in: [nil]` keeps working.

Closes #1653

Co-Authored-By: gdott9 <1830024+gdott9@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Document blank value handling in advanced searches

Records that the shorthand and low-level `c:` forms now agree: a blank
value is dropped in both, so neither leaves a dangling join behind.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Fix cross-link in the advanced mode docs

Docusaurus resolved the URL-style relative link against the page's own
path, so ./configuration#blank-values became
/getting-started/advanced-mode/configuration/ and the docs build failed
on a broken link. Linking by file path lets Docusaurus resolve it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Prune blank advanced conditions at every nesting depth

The pruning only looked at a top-level c:, so the same dangling join
survived when the condition sat inside a g: grouping — which nests to any
depth and is the form Grouping#build accepts. It now descends into every
grouping, under the short or long key spellings.

Search#initialize also only shallow-duplicated the params, so deleting
from a nested conditions hash edited the caller's own object. The params
are deep-duplicated first, and a spec snapshots the input to prove it
comes back unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: gdott9 <1830024+gdott9@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
scarroll32 added a commit that referenced this pull request Sep 21, 2026
* deep strip whitespace in nested search params with group conditions

* fix ActiveModel::RangeError when using multiple search fields

* Respect `config.active_record.permanent_connection_checkout`

* Prune blank conditions from the low-level c: API

A condition given through the `c:` API with an empty value was not
dropped, because the params filter in Search#initialize only looks at
top-level values and `c:` nests its values a level deeper. The condition
still built its attribute and contributed a join, so the query came out
with a LEFT OUTER JOIN and no WHERE clause to go with it — behaving
differently from the equivalent shorthand form.

Builds on @gdott9's work in #1653, with three changes.

The Hash form of `v:` was dropped unconditionally. The original guard
read `value[:value]` where `value` is the enclosing condition hash, so it
was always nil, always blank, and every Hash-form value was pruned:

  Person.ransack(c: { "0" => { a: ["name"], p: "eq",
                               v: { "0" => { value: "Ernie" } } } })
  # => SELECT "people".* FROM "people"    (condition silently gone)

A search that silently returns everything is worse than the bug being
fixed, so the value is now unwrapped from either form before testing it.

`params` is not yet indifferent-access at this point, so a string-keyed
`"c"` — the shape that arrives from an HTTP query string — was missed.
Both spellings are handled.

The blank test is factored out so the top-level filter and the `c:`
filter cannot drift apart, and so `name_in: [nil]` keeps working.

Closes #1653

Co-Authored-By: gdott9 <1830024+gdott9@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Escape LIKE wildcards on every adapter and emit an ESCAPE clause

`escape_wildcards` only escaped `%`, `_` and `\` on MySQL, PostgreSQL and
PostGIS. On SQLite and every other backend it returned the value
untouched, so a `%` or `_` typed by a user acted as a wildcard:
`name_cont: "50%"` matched "50" followed by anything, and `name_cont:
"a_c"` matched "abc".

Escaping alone is not enough to fix this. MySQL and PostgreSQL treat a
backslash as the default LIKE escape character, but SQLite has no default
at all — a backslash there is an ordinary character, so escaping without
an ESCAPE clause just inserts literal backslashes into the pattern.

So escape unconditionally, and pass the escape character through to Arel
so every LIKE / NOT LIKE predicate carries `ESCAPE '\'`. Behaviour is now
identical across adapters and no longer depends on a backend's default.

Two incidental improvements: `.` is no longer escaped on PostgreSQL (it
is not a LIKE wildcard, and escaping it changed nothing semantically),
and the adapter check is gone, which removes a call to the deprecated
`ActiveRecord::Base.connection` from the formatting path.

`like_predicate?` now recognises the `_any` / `_all` compound forms,
which take the same escape argument and were previously missed.

Documents the behaviour under "Wildcards in LIKE predicates".

Fixes #1581

Co-Authored-By: heyjosephme <166889171+heyjosephme@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Add ignore_blank_values config option

Ransack drops a condition whose value is blank, which is what makes an
HTML search form return every record when its fields are left empty
rather than none. For a JSON API that default is usually wrong: there an
empty value is an explicit filter, not an untouched form field, so
`name_eq: ""` should mean "find rows with an empty name" and `id_in: []`
should match nothing rather than being ignored.

Adds `Ransack.options[:ignore_blank_values]`, default true, preserving
today's behaviour. Setting it to false treats blank values as values to
search for. `nil` is ignored either way, so params that were never sent
still do not become conditions.

Three places had to agree: the params filter in Search#initialize, the
default predicate validator, and Predicate#validate, which now treats an
explicitly empty array for an array-wanting predicate as a meaningful
filter. The validator consults the option at search time rather than
capturing it when the predicate is defined, so setting it in an
initializer applies to predicates registered before it runs.

Closes #722
Closes #1664

Co-Authored-By: pekopekopekopayo <pekopekopekopayo@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Document whitespace stripping

strip_whitespace was implemented but undocumented — neither the
initializer example nor any prose mentioned it.

Documents the default, that stripping now applies at every level of the
parameters rather than only the top, and how to turn it off globally or
per search. Examples verified against the spec schema.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Document blank value handling in advanced searches

Records that the shorthand and low-level `c:` forms now agree: a blank
value is dropped in both, so neither leaves a dangling join behind.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Add support for the Trilogy adapter

Trilogy is MySQL, but Ransack's adapter checks matched only "Mysql2", so
Trilogy users silently got the non-MySQL branch: LIKE wildcards were not
escaped, which is the #1581 class of bug.

Widens the adapter checks in escape_wildcards and in the specs, adds a
DB=trilogy connection to the spec schema, and adds a CI job so the claim
is actually tested rather than asserted.

Builds on @navels's #1501, which covered the adapter checks. Without CI
there was no way to know whether Ransack worked under Trilogy; the new
job runs the full suite against it on Rails 8.0 and 8.1.

Closes #1500
Closes #1501

Co-Authored-By: navels <2559156+navels@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Read grouping attributes back through ransack_alias

A form field built from an aliased attribute wrote its value under the
real attribute name but read it back under the alias, so read_attribute
found nothing and the field came back empty on the next request:

  ransack_alias :daddy, :parent_name

  search.parent_name_cont  # => "John"
  search.daddy_cont        # => nil

read_attribute now falls back to resolving the alias when the name it
was given holds nothing: it strips the predicate, asks the context for
the aliased attribute, and retries under the real name. Only reached on
the nil path, so the common case is unaffected.

Closes #689
Closes #1529

Co-Authored-By: DougEdey-Filescom <177950379+DougEdey-Filescom@users.noreply.github.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Fix cross-link in the advanced mode docs

Docusaurus resolved the URL-style relative link against the page's own
path, so ./configuration#blank-values became
/getting-started/advanced-mode/configuration/ and the docs build failed
on a broken link. Linking by file path lets Docusaurus resolve it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Correct the comment on the per-attribute range check

After reduction the node's right side is another predicate, not a
Casted value, so nothing was unwrapped — not merely all but the last.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Cover Rails 7.2 in the Trilogy job and fix the aggregate test command

Rails 7.2 is in the supported matrix and ships the Trilogy adapter, so
the job now runs against it too. The one-liner in CONTRIBUTING.md that
runs every database suite had not gained DB=trilogy, and its wording
still said "all three".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Quote each term of a compound LIKE predicate individually

Widening like_predicate? to the _any / _all forms routed their Array of
terms through Arel::Nodes.build_quoted as a single value, which wraps
the whole Array in one Quoted node. matches_any and friends then call
map on it:

  Person.ransack(name_cont_any: ['50%', 'a_c'])
  # NoMethodError: undefined method 'map' for an instance of Arel::Nodes::Quoted

Nothing in the suite exercised a compound LIKE predicate, so it passed.
Each element is now quoted on its own, and four specs cover cont_any,
cont_all, not_cont_any and start_any by their results, including that
every expanded LIKE carries the ESCAPE clause.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Make the compound LIKE specs independent of each other

Rows created by one example in this file persist into the next, so the
four new specs reusing the same names saw each other's records. Names
are now prefixed per example.

The negative spec also asserted the wrong semantics: not_cont_any
expands to NOT LIKE a OR NOT LIKE b, so a row matching only one term is
correctly included. not_cont_all is the exclusion that was intended.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Keep an explicit blank on typed columns when blanks are meaningful

With ignore_blank_values off, Predicate#validate still cast the value
to the column type before running the validator. For an integer or
boolean column that turned '' into nil, the validator rejected it, and
the condition vanished:

  Person.ransack(parent_id_eq: '').result.to_sql
  # => SELECT "people".* FROM "people"    (every row)

That is the one outcome the option exists to prevent. The validator now
sees the value as given when blanks are meaningful, so the condition is
built. Arel renders the cast nil as IS NULL for eq on integer, boolean
and date columns; inside _in or with a comparison predicate it matches
nothing, which is the safe direction.

Also resolves the merge with main: fields_sort_option landed in the
same region of the configuration file and its docs. The strip_whitespace
lines are left to #1688 so the two branches no longer overlap.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Prune blank advanced conditions at every nesting depth

The pruning only looked at a top-level c:, so the same dangling join
survived when the condition sat inside a g: grouping — which nests to any
depth and is the form Grouping#build accepts. It now descends into every
grouping, under the short or long key spellings.

Search#initialize also only shallow-duplicated the params, so deleting
from a nested conditions hash edited the caller's own object. The params
are deep-duplicated first, and a spec snapshots the input to prove it
comes back unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Prove type_for reads metadata without leasing a connection

The suite-wide :disallowed setting only rejects the deprecated
.connection; it would still pass if type_for used lease_connection,
which is precisely the permanent checkout this change avoids. The new
example releases the pool's connection, calls type_for, and asserts
nothing was leased. It fails against the previous
klass.connection.schema_cache.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Skip the double-quoted sort specs on Trilogy as well as Mysql2

The two fields-sort-option specs assert SQL with double-quoted
identifiers and were guarded against Mysql2 only. Trilogy is MySQL and
emits backticks too, so the new job failed on exactly those two
examples on every leg. The guard now covers both MySQL adapters.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Convert the remaining quoted_true/false calls in the spec suite

Eight ActiveRecord::Base.connection.quoted_true / quoted_false calls in
predicate_spec.rb had survived the conversion; they go through
lease_connection now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Restore the Trilogy widening of the rails7_and_mysql spec helper

Lost while resolving the merge with the LIKE-escaping branch, which took
that file wholesale. Without it the eight scope specs that expect
MySQL-style quoted values ran their SQLite expectations on Trilogy.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Andrii Ukrainets <andrii.ukraiinets@gmail.com>
Co-authored-by: Manh Kha <kha.manhthai@wogi.biz>
Co-authored-by: kha-wogi <148739592+kha-wogi@users.noreply.github.com>
Co-authored-by: Joshua Young <djry1999@gmail.com>
Co-authored-by: gdott9 <1830024+gdott9@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: heyjosephme <166889171+heyjosephme@users.noreply.github.com>
Co-authored-by: pekopekopekopayo <pekopekopekopayo@users.noreply.github.com>
Co-authored-by: aukraine <43291431+aukraine@users.noreply.github.com>
Co-authored-by: joshuay03 <54629302+joshuay03@users.noreply.github.com>
Co-authored-by: navels <2559156+navels@users.noreply.github.com>
Co-authored-by: DougEdey-Filescom <177950379+DougEdey-Filescom@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants