Skip to content
New issue

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

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

Already on GitHub? # to your account

Add support for Zig arrays as MySql values #21

Merged
merged 5 commits into from
Jan 23, 2025

Conversation

evyatark2
Copy link
Contributor

@evyatark2 evyatark2 commented Jan 22, 2025

I am conflicted on how to handle insufficiently-sized arrays.

In this PR, I just truncate the data to fit the array, but in Connector/C they return an error indicating that a short-read was performed.

Would like to hear your opinion on this.

EDIT: Forgot to mention that this should be used for fixed-length MySql columns (i.e. a hash of 16-bytes), so maybe it is a user error to pass an incorrectly-sized array?

@speed2exe
Copy link
Owner

@evyatark2

Thanks for submitting the PR. My preference is about the same as yours (truncating), it's a little implicit, but I guess user should expect this when they are using field with fixed sized array type, so I am fine with it. They would be on their own.

Do you mind adding test cases in integration_tests/conn.zig for scenarios including:

  • len of fixed array is the same as what server returns
  • len of fixed array is greater (some bytes at the end will be undefined garbage)
  • len of fixed array is lesser (truncated)

Also, adding this to README.md:354 will be helpful for documentation

@evyatark2
Copy link
Contributor Author

Sure thing! I am also working on support for std.bounded_array

@evyatark2
Copy link
Contributor Author

Added some tests.
I can't seem to find a good place to document this feature in the README

@speed2exe
Copy link
Owner

@evyatark2 Thanks! The test looks great! I have left one comment. The Types support can be found in the last section: https://github.com/speed2exe/myzql/?tab=readme-ov-file#binary-column-types-support

@speed2exe
Copy link
Owner

LGTM

@evyatark2
Copy link
Contributor Author

I'll create a new PR with documentation for both arrays and BoundedArrays soon

@speed2exe
Copy link
Owner

@evyatark2 PR is looks good. Do you mind resolving the merge conflict?

@evyatark2
Copy link
Contributor Author

Done.

@speed2exe speed2exe merged commit 2b741a1 into speed2exe:main Jan 23, 2025
2 checks passed
@evyatark2 evyatark2 deleted the array branch January 23, 2025 10:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants