-
Notifications
You must be signed in to change notification settings - Fork 491
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
Fix batch queries missing all fields after the first nil field #1320
Conversation
@nathanielc I do now! I've set up a dev environment and run my tests both with and without the PR. Initially looked good, but ran into problems. Certainly appears to fix the first issue in #1294 :
I then went back and tried it against the original report. Also looking good. Observe:
Which gives:
Now:
Then
So all data accounted for and all in order, but all tags have become fields. So next I tried to test the other main sub-issue:
by recording with GROUP BY:
And the call never returns. Alas, it looks like a small step forward and a big step backward. Maybe there was a reason for that Further info: it's the |
@hraftery Thanks for the detailed writeup, I'll take a look at the second set of issues. |
965668f
to
b9f3a7f
Compare
@hraftery I have fixed the second issue. Basically the first issue was masking another hidden bug that caused an infinite loop in the ordering logic for recordings. At this point your test (using GROUP BY *) should work, and the recording should contain all the data in order by time. Can you give it a try when you get a chance? |
Looks great. Quite a process to update and test, but results look good. |
@hraftery Thanks for going through the effort. I'll get this merged and cut a new beta release of 1.3.0 |
Fix batch queries missing all fields after the first nil field
Fixes #1294 , @hraftery do you have the ability to build and test out this PR to confirm it fixes your issue?