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

initial comparison and like calcs based on collation #448

Merged
merged 3 commits into from Jun 3, 2021

Conversation

bheni
Copy link
Contributor

@bheni bheni commented Jun 3, 2021

Currently the LIKE operator always does a case sensitive match. This should be based on the collation. This puts initial support in for doing per collation comparison and like calculations. Currently the comparison changes are disabled as the codebase uses the compare function for more than just WHERE clause calculations which sometimes need to be case sensitive.

Copy link
Contributor

@andy-wm-arthur andy-wm-arthur left a comment

Choose a reason for hiding this comment

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

This looks good, Zach should take a pass as well

@@ -65,6 +65,7 @@ func TestLike(t *testing.T) {
{"a%b", "ab", true},
{"a%b", "a", false},
{"a_b", "ab", false},
{"aa:%", "AA:BB:CC:DD:EE:FF", true},
Copy link
Contributor

Choose a reason for hiding this comment

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

this test could be simpler, why not

{"aa:%", "AA:BB", true},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no reason. Was using a valid MAC address format like what we see in the nautobot code.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh, got it

var CollationToMySQLVals = map[string]mysqlCollationRow{
Collation_binary.Name: {63, Y, Y, 0, NoPad},
Collation_utf8_general_ci.Name: {33, Y, Y, 1, PadSpace},
Collation_utf8mb4_0900_ai_ci.Name: {255, Y, Y, 0, NoPad},
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine for now, given the size of the change already. I think we want CollationToMySQLVals and characterSetMaxLengths, etc. to be inlined into the Collation struct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, true. Not exactly sure what I want to do with the collations structure / interface, but I'll go back and take a second look soon and figure out what to do about case sensitivity in where clauses which is busted atm.

@bheni bheni merged commit 4c1a242 into master Jun 3, 2021
@bheni bheni deleted the bh/collation-changes branch June 3, 2021 22:20
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.

None yet

2 participants