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

Allow raw binary access via GetFieldValue<byte[]> and GetBytes #3639

Closed
roji opened this issue Apr 8, 2021 · 3 comments · Fixed by #5123
Closed

Allow raw binary access via GetFieldValue<byte[]> and GetBytes #3639

roji opened this issue Apr 8, 2021 · 3 comments · Fixed by #5123
Assignees
Milestone

Comments

@roji
Copy link
Member

roji commented Apr 8, 2021

#3152 allowed calling NpgsqlDataReader.GetStream on any data type; this allows e.g. getting a raw string representation for decoding outside of Npgsql, for efficient JSON decoding.

We could allow the same for NpgsqlDataReader.GetFieldValue<byte[]> and NpgsqlDataReader.GetBytes(), which could be useful in certain short value scenarios. We should probably wait to see if someone cares first.

@roji roji added this to the Backlog milestone Apr 8, 2021
@Emill
Copy link
Contributor

Emill commented Apr 13, 2021

I could think that these methods should be called GetRawData or something, to avoid people accidentally getting a byte[] on a column instead of an exception, in case they for example used the wrong column ordinal.

@roji
Copy link
Member Author

roji commented Apr 14, 2021

@Emill I see what you mean, though:

  • We already allow this for GetStream, which seems to be the same case (i.e. no GetRawStream)
  • GetRawData would be Npgsql-specific (whereas GetFieldValue can be used from DbDataReader)
  • NpgsqlDataReader is already a type with many methods, so we shouldn't add more too lightly.
  • It could be claimed that if people accidentally use the raw ordinal, they'd know pretty quickly as the data they get back would not be expected. So IMHO the actual potential for bugs probably doesn't justify adding a new API.

@NinoFloris
Copy link
Member

GetBytes is supported in #5123, adding general GetFieldValue<byte[]> support is probably best left out as it'd be another type check on the hot path.

@NinoFloris NinoFloris mentioned this issue Aug 15, 2023
8 tasks
@NinoFloris NinoFloris modified the milestones: Backlog, 8.0.0 Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants