Skip to content

Commit

Permalink
AVRO-3939: [Rust] Make it possible to use custom schema equality comp…
Browse files Browse the repository at this point in the history
…arators (#2739)

* AVRO-3939: [Rust] Make it possible to use custom schema comparators

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Temporarily use StructFieldComparator as default

Implement compare_fields()

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Fix formatting and clippy issues

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Rename the trait and its impls from Comparator to Eq

Add unit tests for the primitives

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Add support for comparing the custom attributes to
StructFieldEq

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Add more unit tests

Fix clippy error with Rust nightly, e.g.:

```
error: the item `TryFrom` is imported redundantly
  --> avro/src/types.rs:34:5
   |
34 |     convert::TryFrom,
   |     ^^^^^^^^^^^^^^^^
  --> /rustc/2bf78d12d33ae02d10010309a0d85dd04e7cff72/library/std/src/prelude/mod.rs:129:13
   |
   = note: the item `TryFrom` is already defined here

error: could not compile `apache-avro` (lib) due to 9 previous errors
```

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRo-3939: Add more unit tests

fix more Rust nightly clippy errors in tests

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: More unit tests and better logging

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Fix rust nightly clippy error

```
error: the item `RecordField` is imported redundantly
    --> avro/src/types.rs:1153:18
     |
1150 |     use super::*;
     |         -------- the item `RecordField` is already imported here
...
1153 |         schema::{RecordField, RecordFieldOrder},
     |                  ^^^^^^^^^^^
     |
     = note: `-D unused-imports` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(unused_imports)]`
```

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Improve TestLogger's assert_logged to assert against all logged messages

Not just against the last logged message

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* Fix rust nightly clippy error

```
error: the item `FromIterator` is imported redundantly
  --> avro/benches/serde_json.rs:20:33
   |
20 | use std::{collections::HashMap, iter::FromIterator};
   |                                 ^^^^^^^^^^^^^^^^^^
  --> /rustc/2bf78d12d33ae02d10010309a0d85dd04e7cff72/library/std/src/prelude/mod.rs:129:13
   |
   = note: the item `FromIterator` is already defined here
   |
   = note: `-D unused-imports` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(unused_imports)]`

error: could not compile `apache-avro` (bench "serde_json") due to 1 previous error
warning: build failed, waiting for other jobs to finish...
Error: Process completed with exit code 101.
```

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: More test assertions and better logging

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3939: Use StructFieldEq impl by default

It is much faster than SpecificationEq which serializes the schemas to
JSON and then compares them as strings

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVro-3939: Format the code

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

---------

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
  • Loading branch information
martin-g authored Feb 20, 2024
1 parent c2ffa83 commit e038004
Show file tree
Hide file tree
Showing 15 changed files with 623 additions and 30 deletions.
7 changes: 7 additions & 0 deletions lang/rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lang/rust/avro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ md-5 = { default-features = false, version = "0.10.6" }
pretty_assertions = { default-features = false, version = "1.4.0", features = ["std"] }
serial_test = "3.0.0"
sha2 = { default-features = false, version = "0.10.8" }

paste = { default-features = false, version = "1.0.14" }

[package.metadata.docs.rs]
all-features = true
Expand Down
2 changes: 1 addition & 1 deletion lang/rust/avro/benches/serde_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

use criterion::{criterion_group, criterion_main, Criterion};
use serde_json::Value;
use std::{collections::HashMap, iter::FromIterator};
use std::collections::HashMap;

fn make_big_json_record() -> Value {
let address = HashMap::<_, _>::from_iter(vec![
Expand Down
5 changes: 1 addition & 4 deletions lang/rust/avro/src/bigdecimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ pub(crate) fn deserialize_big_decimal(bytes: &Vec<u8>) -> Result<BigDecimal, Err
#[cfg(test)]
mod tests {
use super::*;
use crate::{
types::{Record, Value},
Codec, Reader, Schema, Writer,
};
use crate::{types::Record, Codec, Reader, Schema, Writer};
use apache_avro_test_helper::TestResult;
use bigdecimal::{One, Zero};
use pretty_assertions::assert_eq;
Expand Down
1 change: 0 additions & 1 deletion lang/rust/avro/src/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ mod tests {
use super::*;
use apache_avro_test_helper::TestResult;
use pretty_assertions::assert_eq;
use std::convert::TryFrom;

#[test]
fn test_decimal_from_bytes_from_ref_decimal() -> TestResult {
Expand Down
1 change: 0 additions & 1 deletion lang/rust/avro/src/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use crate::{
use std::{
borrow::Borrow,
collections::HashMap,
convert::TryFrom,
io::{ErrorKind, Read},
str::FromStr,
};
Expand Down
7 changes: 1 addition & 6 deletions lang/rust/avro/src/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ use crate::{
util::{zig_i32, zig_i64},
AvroResult, Error,
};
use std::{
borrow::Borrow,
collections::HashMap,
convert::{TryFrom, TryInto},
};
use std::{borrow::Borrow, collections::HashMap};

/// Encode a `Value` into avro format.
///
Expand Down Expand Up @@ -311,7 +307,6 @@ pub(crate) mod tests {
use super::*;
use apache_avro_test_helper::TestResult;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use uuid::Uuid;

pub(crate) fn success(value: &Value, schema: &Schema) -> String {
Expand Down
1 change: 1 addition & 0 deletions lang/rust/avro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ mod writer;
pub mod rabin;
pub mod schema;
pub mod schema_compatibility;
pub mod schema_equality;
pub mod types;
pub mod validator;

Expand Down
3 changes: 1 addition & 2 deletions lang/rust/avro/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use serde::de::DeserializeOwned;
use serde_json::from_slice;
use std::{
collections::HashMap,
convert::TryFrom,
io::{ErrorKind, Read},
marker::PhantomData,
str::FromStr,
Expand Down Expand Up @@ -528,7 +527,7 @@ pub fn read_marker(bytes: &[u8]) -> [u8; 16] {
#[cfg(test)]
mod tests {
use super::*;
use crate::{encode::encode, from_value, types::Record, Reader};
use crate::{encode::encode, types::Record};
use apache_avro_test_helper::TestResult;
use pretty_assertions::assert_eq;
use serde::Deserialize;
Expand Down
8 changes: 4 additions & 4 deletions lang/rust/avro/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! Logic for parsing and interacting with schemas in Avro format.
use crate::{
error::Error,
types,
schema_equality, types,
util::MapHelper,
validator::{
validate_enum_symbol_name, validate_namespace, validate_record_field_name,
Expand All @@ -35,7 +35,6 @@ use serde_json::{Map, Value};
use std::{
borrow::{Borrow, Cow},
collections::{BTreeMap, HashMap, HashSet},
convert::{TryFrom, TryInto},
fmt,
fmt::Debug,
hash::Hash,
Expand Down Expand Up @@ -155,9 +154,9 @@ impl PartialEq for Schema {
/// Assess equality of two `Schema` based on [Parsing Canonical Form].
///
/// [Parsing Canonical Form]:
/// https://avro.apache.org/docs/1.8.2/spec.html#Parsing+Canonical+Form+for+Schemas
/// https://avro.apache.org/docs/1.11.1/specification/#parsing-canonical-form-for-schemas
fn eq(&self, other: &Self) -> bool {
self.canonical_form() == other.canonical_form()
schema_equality::compare_schemata(self, other)
}
}

Expand Down Expand Up @@ -6356,6 +6355,7 @@ mod tests {
"logicalType": "uuid"
});
let parse_result = Schema::parse(&schema)?;

assert_eq!(
parse_result,
Schema::Fixed(FixedSchema {
Expand Down
Loading

0 comments on commit e038004

Please sign in to comment.