Skip to content

Commit 75adf1f

Browse files
fix: add medium-level validation guards for facets and tokenizer args (#28)
Co-authored-by: Ankit <573824+ankitml@users.noreply.github.com>
1 parent b515af1 commit 75adf1f

4 files changed

Lines changed: 71 additions & 3 deletions

File tree

lib/parade_db/migration_helpers.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,11 @@ def tokenizer_args(options)
213213
positional = Array(opts.delete(:__positional)).map { |value| tokenizer_positional_arg_sql(value) }
214214

215215
if opts.key?(:min) || opts.key?(:max)
216-
if opts.key?(:min) && opts.key?(:max)
217-
positional << Integer(opts.delete(:min)).to_s
218-
positional << Integer(opts.delete(:max)).to_s
216+
unless opts.key?(:min) && opts.key?(:max)
217+
raise ArgumentError, "tokenizer named args :min and :max must be provided together"
219218
end
219+
positional << Integer(opts.delete(:min)).to_s
220+
positional << Integer(opts.delete(:max)).to_s
220221
end
221222

222223
named = opts.map do |k, v|

lib/parade_db/search_methods.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,9 +599,24 @@ def normalize_facet_fields(fields, agg:)
599599
raise ArgumentError, "Facet field names must be unique."
600600
end
601601

602+
validate_facet_fields_indexed!(normalized)
602603
normalized
603604
end
604605

606+
def validate_facet_fields_indexed!(fields)
607+
return unless klass.respond_to?(:paradedb_indexed_fields)
608+
609+
indexed_fields = klass.paradedb_indexed_fields
610+
return if indexed_fields.empty?
611+
612+
unknown = fields.reject { |field| indexed_fields.include?(field) }
613+
return if unknown.empty?
614+
615+
raise ParadeDB::FieldNotIndexed,
616+
"#{klass.name}.facets contains non-indexed fields #{unknown.join(', ')}. " \
617+
"Indexed fields: #{indexed_fields.join(', ')}"
618+
end
619+
605620
def normalize_facet_size(size)
606621
return nil if size.nil?
607622

spec/index_dsl_unit_spec.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,38 @@
141141
assert_includes sql, "pdb.ngram(2, 5)"
142142
end
143143

144+
it "rejects partial ngram bounds in named_args" do
145+
klass = Class.new(ParadeDB::Index) do
146+
self.table_name = :products
147+
self.key_field = :id
148+
self.fields = {
149+
id: {},
150+
description: { tokenizer: :ngram, named_args: { min: 2 } }
151+
}
152+
end
153+
154+
error = assert_raises(ArgumentError) do
155+
ActiveRecord::Base.connection.send(:build_create_sql, klass.compiled_definition, if_not_exists: false)
156+
end
157+
assert_includes error.message, ":min and :max must be provided together"
158+
end
159+
160+
it "rejects max-only ngram bounds in named_args" do
161+
klass = Class.new(ParadeDB::Index) do
162+
self.table_name = :products
163+
self.key_field = :id
164+
self.fields = {
165+
id: {},
166+
description: { tokenizer: :ngram, named_args: { max: 5 } }
167+
}
168+
end
169+
170+
error = assert_raises(ArgumentError) do
171+
ActiveRecord::Base.connection.send(:build_create_sql, klass.compiled_definition, if_not_exists: false)
172+
end
173+
assert_includes error.message, ":min and :max must be provided together"
174+
end
175+
144176
it "renders qualified and inline tokenizer forms" do
145177
klass = Class.new(ParadeDB::Index) do
146178
self.table_name = :products

spec/index_runtime_features_unit_spec.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,26 @@
126126
assert_includes sql, "::pdb.alias('description_simple')"
127127
end
128128

129+
it "facets raises FieldNotIndexed for non-indexed facet fields" do
130+
Object.const_set("RuntimeProduct", Class.new(ActiveRecord::Base) do
131+
self.table_name = :products
132+
include ParadeDB::Model
133+
end)
134+
Object.const_set("RuntimeProductIndex", Class.new(ParadeDB::Index) do
135+
self.table_name = :products
136+
self.key_field = :id
137+
self.fields = { id: {}, description: {} }
138+
end)
139+
140+
conn = ActiveRecord::Base.connection
141+
conn.create_paradedb_index(RuntimeProductIndex, if_not_exists: true)
142+
143+
error = assert_raises(ParadeDB::FieldNotIndexed) do
144+
RuntimeProduct.search(:description).matching_all("shoe").build_facet_query(fields: [:price]).sql
145+
end
146+
assert_includes error.message, "non-indexed fields"
147+
end
148+
129149
it "raise mode detects missing catalog index drift" do
130150
Object.const_set("DriftProduct", Class.new(ActiveRecord::Base) do
131151
self.table_name = :products

0 commit comments

Comments
 (0)