-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add support for doing binary encoding of result rows for clients that send that info (like PowerBI) #23
Conversation
… send this info (like PowerBI)
Hi, I just noticed the new branch. I merged those changes into my fork under branch dev. I am getting index out of range errors around query_result.result_format[i] under send_row_description Also due to the error, the Power BI connectivity is broken. You can clone my fork (dev branch) and try out the example Power BI file. Should work out of the box (on main branch) or can be debugged (on dev). |
Wow. I am learning more of git now. My dev branch brought all you commits in jwills_powerbi branch except the changes to requirements file. The first commit in dev branch (I did it manually through vscode compare files and clicking the arrow to bring in your changes). Other commits I cherry picked using git and I synced with github. The out of range issue is still there! |
Ha-- love to see it, thanks so much for giving it a try! Apologies for the lag on my side, I have a couple of other things in flight right now but will get back to this on Monday! |
No problem. Happy that you are interested too. First time I engage with someone on github. Once this fixed, I guess Power BI + DuckDB (Direct Query) will be in a very good shape for production usage. |
@actuary87 okay I spent several hours on this today and I think I've got most of the issues sorted out; would you give it another crack when you have some time? 🙇 |
Thanks a lot. I still get the errors. I will try to debug to understand. I noted one thing, I hope it's a good sign: Although Power Query had errors (unable to connect to db) but when I opened Power BI today the cache version of the table surprisingly showed integer numbers. If it means something it means somehow it was able to parse binary format before the error! I hope so. Will give it a debug try and let you know. |
Thank you very much for the changes. I did some of debugging during the weekend and I managed to solve the errors I reported earlier. So your changes works well, but I need to test all data types before I fully conclude. Power Query don't report errors now :D With regards to Numeric data, Integers are ok but Decimal does not work unless I send it as TEXT (changing OID to 25 for both Decimal and Unknown types). It'll be good if I know how Postgresql sends the Decimal to the client. I can't seem to find the proper document which explains this. Do you have something to share in this regard? I will push changes in my fork and merge them to main once the testing goes well. |
Hey @actuary87, always a pleasure to hear from you! Let me see if I can figure out the Decimal encoding format, it would obviously be amazing if that is all that is holding us back now! |
Going to merge this for now to get it out there; getting the decimal binary protocol correct is going to be some work/take some time to get it right |
I probably don't have the details of all of the different formats correct yet but will iterate on them here