Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add postgres macaddr8 type support #4313

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

daniele-solutiontech
Copy link

No description provided.

@weiznich weiznich requested a review from a team October 19, 2024 17:59
Copy link
Member

@weiznich weiznich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this PR. Overall this looks ok to me, but there are a few minor things that currently make the tests fail to compile. These need to be fixed. You also need to run the test locally at least once to update a view snapshots. You can do this via cargo xtask run-tests postgres.

You should also run cargo fmt locally before submitting the PR.
Finally it would be great to have a changelog entry for this.

@@ -99,6 +99,7 @@ fn pg_diesel_types() -> HashSet<&'static str> {
types.insert("Inet");
types.insert("Jsonb");
types.insert("MacAddr");
types.insert("MacAddr8");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are missing another insert below line 130 for the lowercase variant.

/// .values(macaddr.eq([0x08, 0x00, 0x2b, 0x01, 0x02, 0x03, 0x04, 0x05]))
/// .returning(macaddr)
/// .get_result::<[u8; 8]>(connection)?;
/// assert_eq!([0x08, 0x00, 0x2b, 0x01, 0x02, 0x03, 0x04, 0x0], inserted_macaddr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// assert_eq!([0x08, 0x00, 0x2b, 0x01, 0x02, 0x03, 0x04, 0x0], inserted_macaddr);
/// assert_eq!([0x08, 0x00, 0x2b, 0x01, 0x02, 0x03, 0x04, 0x05], inserted_macaddr);

let expected_value = [0x08, 0x00, 0x2b, 0x01, 0x02, 0x03, 0x04, 0x05];
assert_eq!(
expected_value,
query_single_value::<MacAddr, [u8; 8]>(query)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
query_single_value::<MacAddr, [u8; 8]>(query)
query_single_value::<MacAddr8, [u8; 8]>(query)

fn pg_macaddress8_to_sql_macaddress() {
let expected_value = "'08:00:2b:01:02:03:04:05'::macaddr8";
let value = [0x08, 0x00, 0x2b, 0x01, 0x02, 0x03, 0x04, 0x05];
assert!(query_to_sql_equality::<MacAddr, [u8; 8]>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert!(query_to_sql_equality::<MacAddr, [u8; 8]>(
assert!(query_to_sql_equality::<MacAddr8, [u8; 8]>(

macaddr8_roundtrips,
MacAddr8,
(u8, u8, u8, u8, u8, u8, u8, u8),
mk_macaddr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mk_macaddr is a function below in this file. We need to have a mk_macaddr8 variant there as well.

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