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

Using tsv reader #114

Merged
merged 9 commits into from
Apr 12, 2021
Merged

Using tsv reader #114

merged 9 commits into from
Apr 12, 2021

Conversation

glebvk85
Copy link
Contributor

@glebvk85 glebvk85 commented Apr 8, 2021

Reading lines as TSV, instead of using CSV.

Fixes bugs
#103
#110

@@ -81,6 +82,15 @@ func (r *textRows) Next(dest []driver.Value) error {
return err
}

// skip row before WITH TOTALS,
// not but do not skip an empty line if it is part of the result
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please elaborate a little more. I don't quite understand the meaning

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we read a line that contains nothing (condition len(row) == 1 && row[0] == ""), then two options are possible:

  1. SELECT query ''
    We need to keep this line as part of the output:
    Let's check that we requested one item
    len(dest)! = 1 - the condition is not met, do not skip this data

  2. This is a line break in queries like WITH TOTALS, WITH CUBE, WITH ROLLUP, where the aggregated part is separated from the main empty line:
    It is important that such queries require groupings: WITH TOTALS, ROLLUP or CUBE are not supported without aggregation
    In such queries, there will be more than one column, or it will not be empty. We skip this line in the results.

@@ -54,7 +54,7 @@ func TestTextRows(t *testing.T) {
}

func TestTextRowsQuoted(t *testing.T) {
buf := bytes.NewReader([]byte("text\nArray(String)\n['Quote: \"here\"']"))
buf := bytes.NewReader([]byte("text\nArray(String)\n['Quote: \"here\"']\n"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does clickhouse return the data with a trailing \n? if not we should leave the tests as is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, clickhouse output with the TabSeparatedWithNamesAndTypes format ends with \n

@DoubleDi
Copy link
Collaborator

DoubleDi commented Apr 8, 2021

Hi! Thanks for PR.
Can you please add some tests for the specific case of the double quotes errors from the issues, you mentioned.

@glebvk85 glebvk85 requested a review from DoubleDi April 12, 2021 13:14
@DoubleDi
Copy link
Collaborator

Thanks for the great work!

@DoubleDi DoubleDi merged commit c301c6f into mailru:master Apr 12, 2021
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.

None yet

2 participants