Panic on duplicate field names

fixes #1601
quickwit-oss · Oct 26, 2022 · 81b4691 · 81b4691
1 parent a5e59ab
commit 81b4691
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 49 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -819,7 +819,7 @@ pub mod tests {
     fn test_indexedfield_not_in_documents() -> crate::Result<()> {
         let mut schema_builder = Schema::builder();
         let text_field = schema_builder.add_text_field("text", TEXT);
-        let absent_field = schema_builder.add_text_field("text", TEXT);
+        let absent_field = schema_builder.add_text_field("absent_text", TEXT);
         let schema = schema_builder.build();
         let index = Index::create_in_ram(schema);
         let mut index_writer = index.writer_for_tests()?;
@@ -1001,7 +1001,7 @@ pub mod tests {
         let fast_field_signed = schema_builder.add_i64_field("signed", FAST);
         let fast_field_float = schema_builder.add_f64_field("float", FAST);
         let text_field = schema_builder.add_text_field("text", TEXT);
-        let stored_int_field = schema_builder.add_u64_field("text", STORED);
+        let stored_int_field = schema_builder.add_u64_field("stored_int", STORED);
         let schema = schema_builder.build();
 
         let index = Index::create_in_ram(schema);

diff --git a/src/query/set_query.rs b/src/query/set_query.rs
@@ -115,7 +115,7 @@ mod tests {
     pub fn test_term_set_query() -> crate::Result<()> {
         let mut schema_builder = Schema::builder();
         let field1 = schema_builder.add_text_field("field1", TEXT);
-        let field2 = schema_builder.add_text_field("field1", TEXT);
+        let field2 = schema_builder.add_text_field("field2", TEXT);
         let schema = schema_builder.build();
         let index = Index::create_in_ram(schema);
         {

diff --git a/src/schema/schema.rs b/src/schema/schema.rs
@@ -46,13 +46,9 @@ impl SchemaBuilder {
     /// Adds a new u64 field.
     /// Returns the associated field handle
     ///
-    /// # Caution
+    /// # Panics
     ///
-    /// Appending two fields with the same name
-    /// will result in the shadowing of the first
-    /// by the second one.
-    /// The first field will get a field id
-    /// but only the second one will be indexed
+    /// Panics when field already exists.
     pub fn add_u64_field<T: Into<NumericOptions>>(
         &mut self,
         field_name_str: &str,
@@ -66,13 +62,9 @@ impl SchemaBuilder {
     /// Adds a new i64 field.
     /// Returns the associated field handle
     ///
-    /// # Caution
+    /// # Panics
     ///
-    /// Appending two fields with the same name
-    /// will result in the shadowing of the first
-    /// by the second one.
-    /// The first field will get a field id
-    /// but only the second one will be indexed
+    /// Panics when field already exists.
     pub fn add_i64_field<T: Into<NumericOptions>>(
         &mut self,
         field_name_str: &str,
@@ -86,13 +78,9 @@ impl SchemaBuilder {
     /// Adds a new f64 field.
     /// Returns the associated field handle
     ///
-    /// # Caution
+    /// # Panics
     ///
-    /// Appending two fields with the same name
-    /// will result in the shadowing of the first
-    /// by the second one.
-    /// The first field will get a field id
-    /// but only the second one will be indexed
+    /// Panics when field already exists.
     pub fn add_f64_field<T: Into<NumericOptions>>(
         &mut self,
         field_name_str: &str,
@@ -106,13 +94,9 @@ impl SchemaBuilder {
     /// Adds a new bool field.
     /// Returns the associated field handle
     ///
-    /// # Caution
+    /// # Panics
     ///
-    /// Appending two fields with the same name
-    /// will result in the shadowing of the first
-    /// by the second one.
-    /// The first field will get a field id
-    /// but only the second one will be indexed
+    /// Panics when field already exists.
     pub fn add_bool_field<T: Into<NumericOptions>>(
         &mut self,
         field_name_str: &str,
@@ -128,13 +112,9 @@ impl SchemaBuilder {
     /// Internally, Tantivy simply stores dates as i64 UTC timestamps,
     /// while the user supplies DateTime values for convenience.
     ///
-    /// # Caution
+    /// # Panics
     ///
-    /// Appending two fields with the same name
-    /// will result in the shadowing of the first
-    /// by the second one.
-    /// The first field will get a field id
-    /// but only the second one will be indexed
+    /// Panics when field already exists.
     pub fn add_date_field<T: Into<DateOptions>>(
         &mut self,
         field_name_str: &str,
@@ -148,13 +128,9 @@ impl SchemaBuilder {
     /// Adds a ip field.
     /// Returns the associated field handle.
     ///
-    /// # Caution
+    /// # Panics
     ///
-    /// Appending two fields with the same name
-    /// will result in the shadowing of the first
-    /// by the second one.
-    /// The first field will get a field id
-    /// but only the second one will be indexed
+    /// Panics when field already exists.
     pub fn add_ip_addr_field<T: Into<IpAddrOptions>>(
         &mut self,
         field_name_str: &str,
@@ -168,13 +144,9 @@ impl SchemaBuilder {
     /// Adds a new text field.
     /// Returns the associated field handle
     ///
-    /// # Caution
+    /// # Panics
     ///
-    /// Appending two fields with the same name
-    /// will result in the shadowing of the first
-    /// by the second one.
-    /// The first field will get a field id
-    /// but only the second one will be indexed
+    /// Panics when field already exists.
     pub fn add_text_field<T: Into<TextOptions>>(
         &mut self,
         field_name_str: &str,
@@ -228,8 +200,10 @@ impl SchemaBuilder {
     pub fn add_field(&mut self, field_entry: FieldEntry) -> Field {
         let field = Field::from_field_id(self.fields.len() as u32);
         let field_name = field_entry.name().to_string();
+        if let Some(_previous_value) = self.fields_map.insert(field_name, field) {
+            panic!("Field already exists in schema {}", field_entry.name());
+        };
         self.fields.push(field_entry);
-        self.fields_map.insert(field_name, field);
         field
     }
 
@@ -388,7 +362,9 @@ impl Schema {
 
 impl Serialize for Schema {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where S: Serializer {
+    where
+        S: Serializer,
+    {
         let mut seq = serializer.serialize_seq(Some(self.0.fields.len()))?;
         for e in &self.0.fields {
             seq.serialize_element(e)?;
@@ -399,7 +375,9 @@ impl Serialize for Schema {
 
 impl<'de> Deserialize<'de> for Schema {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where D: Deserializer<'de> {
+    where
+        D: Deserializer<'de>,
+    {
         struct SchemaVisitor;
 
         impl<'de> Visitor<'de> for SchemaVisitor {
@@ -410,7 +388,9 @@ impl<'de> Deserialize<'de> for Schema {
             }
 
             fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
-            where A: SeqAccess<'de> {
+            where
+                A: SeqAccess<'de>,
+            {
                 let mut schema = SchemaBuilder {
                     fields: Vec::with_capacity(seq.size_hint().unwrap_or(0)),
                     fields_map: HashMap::with_capacity(seq.size_hint().unwrap_or(0)),