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

Need Row.opIndex(string) #168

Open
Abscissa opened this issue Feb 25, 2018 · 6 comments
Open

Need Row.opIndex(string) #168

Abscissa opened this issue Feb 25, 2018 · 6 comments

Comments

@Abscissa
Copy link

Abscissa commented Feb 25, 2018

Been itching for this one for years. Need to finally get it in.

The information needed to lookup columns by name should already be there (in ResultRange). Just need to stash the column into into Row(UPDATE: Already done as of #188 and v2.3.0) and the rest should be trivial.

@Abscissa Abscissa added this to the v2.2.0 milestone Feb 26, 2018
@Abscissa Abscissa modified the milestones: v2.2.0, v2.3.0 Mar 3, 2018
@schveiguy
Copy link
Collaborator

Just because it was asked about today, Row should also have an asAA member, no reason to make you use the ResultRange now that Row has column names. Also, I'd question the wisdom of exposing opIndex(string) as this is going to be O(n), unless you sort the columns and do a binary search (at which point, you may as well just do an AA).

@Abscissa
Copy link
Author

I think what I'd had in mind was to just use an AA internally and ditch asAA (or just leave it for backwards compatibility), but now that I think about it, that would mean behind-the-scenes heap allocations even when name-based access isn't used. (Damn...)

Good point about adding asAA to Row, though.

@Abscissa
Copy link
Author

OTOH, I would think that in the vast majority of cases, the "n" in O(n) would be small enough to be negligible (even when combined with the total number of O(n) lookups also being "n"). Combine that with a note in the docs about it being O(n) ("so therefore don't use it if you're returning a huge number of columns"), and it might not be so bad. Albeit, not ideal.

Maybe there's some clever way to use templating to make the cost of the "internal AA" solution be pay-as-you-go? I just find it such a pain, and horrible UI, to be accessing columns by index instead of name. There's got to be a good solution for this...

@schveiguy
Copy link
Collaborator

schveiguy commented Dec 10, 2019

One "clever" way to do it may be to use an accessor for the map, which can be O(n). So something like:

auto row = conn.queryRow(...);
auto val = row[row.col("id")]; // in-line fetching of column index O(n)
auto idcol = row.col("id"); // cache it O(n) lookup
val = row[colid]; // O(1) lookup

Honestly, I hate dealing with Variant (or even Algebraic), and much rather would deal with a serialized type. My internal library consists mainly of a byItem wrapper that works like:

foreach(MyType t; conn.query(...).byItem!MyType)
{
   auto val = t.id; // typed correctly, easy access.
}

Behind the scenes, byItem figures out the mapping of column index to field name, and after the first setup is complete, everything is mapped for O(1) conversion.

I might publish this some day, but it's pretty undocumented and probably not ready for public consumption.

If you want to do it like all other libraries with an opIndex on the name, the best way is likely a relatively static AA mapping generated once when the sequence is fetched. Since you are already generating GC data (you are already creating an array of strings for the column names). So maybe this isn't so bad. The only thing that sucks is that the builtin AA allocates a lot of little things (one per item), whereas we only really need a static and internal lookup table. I wonder if there is something already out there. At the very least we can kind of duplicate what AA's do, but make it into a simple array instead.

@schveiguy
Copy link
Collaborator

schveiguy commented Dec 10, 2019

The only thing that sucks is that the builtin AA allocates a lot of little things (one per item), whereas we only really need a static and internal lookup table. I wonder if there is something already out there. At the very least we can kind of duplicate what AA's do, but make it into a simple array instead.

I threw this together. What do you think? https://github.com/schveiguy/lookuptable

@schveiguy
Copy link
Collaborator

I'm going to look into including lookuptable for this. I think the functionality would allow for a nice medium ground (have been using lookuptable in my project for some time).

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