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

Add text format result support #961

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ZENOTME
Copy link

@ZENOTME ZENOTME commented Oct 31, 2022

What this PR do

This PR add the TEXT format support for query result. In addition, this modification is compatitable for previous version. User needn't to change the original query.

Context

The rust-postgres only support binary format query result in extended query mode before. But in some cases, binary format of some type(such as multidimensional list) isn't support well. Sometimes, TEXT format result is more convenient. Such as in sqllogictest(It is a testing framework to verify the correctness of an SQL database), it can test a database using extended query protocol. In this case, TEXT format is more convenient to compare the actual result with the expect result.

I try to support text format in this PR. To make it compatible with the previous version, I add a result_format flag in client. The query will send message according to the result_format. And add the get_text in row to get the result as TEXT format.

Some problem may need to solved

  • Row can't guarantee the get_text will caused panic or error if user use it in a 'binary format row'. I only add the comment to indicate that user must guarantee use get_text in 'text format row'.
  • I add the test case but I'm not sure whether enough.

Anyway, I'm glad to make any modification and hope it can be merged into main version.

related issue: #882

@tobyhede
Copy link

+1 from me - our use case needs to support the text format.

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

Successfully merging this pull request may close these issues.

2 participants