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

New Feature: Add Binary[] Load function when Primary Key is Binary[] #1421

Open
xux1217 opened this issue Oct 21, 2024 · 4 comments
Open

New Feature: Add Binary[] Load function when Primary Key is Binary[] #1421

xux1217 opened this issue Oct 21, 2024 · 4 comments

Comments

@xux1217
Copy link

xux1217 commented Oct 21, 2024

What version of SQLBoiler are you using (sqlboiler --version)?

4.16.2

What is your database and version (eg. Postgresql 10)

mysql8

If this happened at generation time what was the full SQLBoiler command you used to generate your models? (if not applicable leave blank)

As this issue(#1381), when primary key is binary, the load function would report error.

The core reason is we use map as the record structure instead of array when release V4.16.0 (https://github.com/volatiletech/sqlboiler/releases/tag/v4.16.0).

Further information. What did you do, what did you expect?

We have many projects use sqlboiler, and we use the load function too, we also need high performance load function, but we have many tables has binary primary key.

The new version breaks the compatibility, so in the future, we also can't upgrade it.

Resolve method?

I think we need to optimize this point, it not very hard.

Just need to do binary sorting as map key when detect the key is binary array.

  1. cover it to string.
  2. implement a special struct(like type sortbinary []byte), implement sort function, and do the type conversion.
@xux1217
Copy link
Author

xux1217 commented Oct 21, 2024

I roughly looked at the template file, is it just necessary to modify this scope(https://github.com/volatiletech/sqlboiler/blob/master/templates/main/07_relationship_to_one_eager.go.tpl#L40) ?

Can we replace the map key type and when converting to an array, the type can be restored.

@stephenafamo
Copy link
Collaborator

I'll be happy to review a PR fixing this.

The new version breaks the compatibility, so in the future, we also can't upgrade it.

Why will the new version break compatibility?
As far as the function signature does not change, the implementation of the Load() function should be able to change.

Or am I missing something?

@xux1217
Copy link
Author

xux1217 commented Oct 24, 2024

I'll be happy to review a PR fixing this.

The new version breaks the compatibility, so in the future, we also can't upgrade it.

Why will the new version break compatibility? As far as the function signature does not change, the implementation of the Load() function should be able to change.

Or am I missing something?

If we fix the crash of the load method for binary primary keys, there will be no compatibility issues.

@xux1217
Copy link
Author

xux1217 commented Oct 24, 2024

#1422

A PR has been submitted. Please help me check whether the modification is appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants