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

should SQLText store the text data as a ByteString instead of [Char]? #3

Closed
nurpax opened this issue Aug 12, 2012 · 5 comments
Closed
Labels
Milestone

Comments

@nurpax
Copy link
Collaborator

nurpax commented Aug 12, 2012

I see that SQLText stores its contents as a String. This may be a problem for performance.

Would it be possible to change it to ByteString instead? (I realize this is an API change.)

It's easy to go from ByteString to String, but going from String to ByteString means increased memory thrashing.

I realize that in some cases it's possible to work-around this by using SQLBlob instead. But using the "columns" function after "step" means many fields often get converted to Strings.

@IreneKnapp
Copy link
Owner

Yes, this is a good idea and I'll do it hopefully today.

On Sun, Aug 12, 2012 at 8:55 AM, Janne Hellsten notifications@github.comwrote:

I see that SQLText stores its contents as a String. This may be a problem
for performance.

Would it be possible to change it to ByteString instead? (I realize this
is an API change.)

It's easy to go from ByteString to String, but going from String to
ByteString means increased memory thrashing.

I realize that in some cases it's possible to work-around this by using
SQLBlob instead. But using the "columns" function after "step" means many
fields often get converted to Strings.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3.

-- Irene Knapp

@IreneKnapp
Copy link
Owner

Actually now that I think about it, how would you feel about it using Text?

Sent from my iPhone

On Aug 12, 2012, at 8:55 AM, Janne Hellsten notifications@github.com wrote:

I see that SQLText stores its contents as a String. This may be a problem for performance.

Would it be possible to change it to ByteString instead? (I realize this is an API change.)

It's easy to go from ByteString to String, but going from String to ByteString means increased memory thrashing.

I realize that in some cases it's possible to work-around this by using SQLBlob instead. But using the "columns" function after "step" means many fields often get converted to Strings.


Reply to this email directly or view it on GitHub.

@nurpax
Copy link
Collaborator Author

nurpax commented Aug 12, 2012

I think both are fine. I've used ByteStrings throughout sqlite-simple now, but that's mainly because mysql-simple and postgresql-simple both used ByteStrings for everything (and a bit of Text too for utf8 conversion). I suppose sticking to ByteStrings would mean the user-facing API would depend on one less package/API. But I don't know how big of a worry that is, since my package requires text anyway.

I think I'll ask for convention guidance on database-devel.

@IreneKnapp
Copy link
Owner

If the two choices are even enough that we need to ask for community opinion, I guess that means you won't mind if I use my status as dictator of this particular package to declare that we'll use Text? Cause I'm doing that. :) I don't think the Text package is an exotic dependency; it deserves to be used pretty widely, since it's solid work and well-maintained. Mostly I'm choosing this because it means the user doesn't need to be concerned with how the text is actually encoded, once they have it out of the database; that's a useful property that they have with String and Text but not with ByteString.

@nurpax
Copy link
Collaborator Author

nurpax commented Aug 13, 2012

Agreed. I'm looking forward to this change, ideally I would cut sqlite-simple over to the new API before my first release.

On 13.8.2012, at 1.43, Irene Knapp notifications@github.com wrote:

If the two choices are even enough that we need to ask for community opinion, I guess that means you won't mind if I use my status as dictator of this particular package to declare that we'll use Text? Cause I'm doing that. :) I don't think the Text package is an exotic dependency; it deserves to be used pretty widely, since it's solid work and well-maintained. Mostly I'm choosing this because it means the user doesn't need to be concerned with how the text is actually encoded, once they have it out of the database; that's a useful property that they have with String and Text but not with ByteString.


Reply to this email directly or view it on GitHub.

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

No branches or pull requests

2 participants