HTTP Part 2

[ortyomka]

Issue #597

Changes

• Add HTTP url options
• Add HTTP auth options
• Add HTTP async insert

cc @gingerwizard

[ortyomka]

I have problems with temp tables. They are stored within the same session, in HTTP the session is indicated by the session_id parameter. In Native, Conn and Tx are separated by session, but in HTTP, they are not. To separate, you need to explicitly specify session_id in context.

Is it worth separating them or specifying context is ok?

Example here

[ortyomka]

I have fixed 2 race conditions:

1. I used shared err. Change os.Pipe to io.Pipe and use CloseWithErr
2. Shared Decoder is second race condition. One request reads a response, but other already change source of reading. To make them independent, return creating of Decoder for each request.

[ortyomka]

@gingerwizard need advice. Each request can read 3 000 000 rows, with shared decode. If the request will be larger, then a result will be incorrect.

What better: shared decode and wait group with rows limit
or
decode for each request, without limits?

[gingerwizard]

@ortyomka will get to this today. soz been heads down on #646 which will impact this PR. It should fix your shared decoder issue though.

[ortyomka]

@gingerwizard, okay. Do I need to revert creation of decoder on each request?

[gingerwizard]

@ortyomka i think we will need to create a reader on each request. see #646 - I don't see anyway to reuse one.

[gingerwizard]

@ortyomka getting some failures in #646 around HTTP

=== RUN   TestSimpleHttpStdArray/Http_Interface
    array_test.go:68: 
        	Error Trace:	/opt/clickhouse-go/tests/std/array_test.go:68
        	Error:      	Received unexpected error:
        	            	read:
        	            	    github.com/ClickHouse/ch-go/proto.(*Reader).UVarInt
        	            	        /Users/dalemcdiarmid/go/pkg/mod/github.com/!click!house/[email protected]/proto/reader.go:92
        	            	  - read:
        	            	    github.com/ClickHouse/ch-go/proto.(*Reader).ReadFull
        	            	        /Users/dalemcdiarmid/go/pkg/mod/github.com/!click!house/[email protected]/proto/reader.go:66
        	            	  - unexpected EOF
        	Test:       	TestSimpleHttpStdArray/Http_Interface
    --- FAIL: TestSimpleHttpStdArray/Http_Interface (425.55s)

This doesn't include this PR but it seems we fail on trying to read the last block with the new reader - unclear why this isn't the case on the old decoder.

[gingerwizard]

@ortyomka i think we need to set Packet on the block to ensure we know when to stop looping on Next()...looking to see where we can get that in the HTTP interface. This isn't being set and we're reading over the buffer.

[ortyomka]

@ortyomka getting some failures in #646 around HTTP

=== RUN   TestSimpleHttpStdArray/Http_Interface
    array_test.go:68: 
        	Error Trace:	/opt/clickhouse-go/tests/std/array_test.go:68
        	Error:      	Received unexpected error:
        	            	read:
        	            	    github.com/ClickHouse/ch-go/proto.(*Reader).UVarInt
        	            	        /Users/dalemcdiarmid/go/pkg/mod/github.com/!click!house/[email protected]/proto/reader.go:92
        	            	  - read:
        	            	    github.com/ClickHouse/ch-go/proto.(*Reader).ReadFull
        	            	        /Users/dalemcdiarmid/go/pkg/mod/github.com/!click!house/[email protected]/proto/reader.go:66
        	            	  - unexpected EOF
        	Test:       	TestSimpleHttpStdArray/Http_Interface
    --- FAIL: TestSimpleHttpStdArray/Http_Interface (425.55s)

This doesn't include this PR but it seems we fail on trying to read the last block with the new reader - unclear why this isn't the case on the old decoder.

I will see, today

[gingerwizard]

New reader is wrapping the eof..meaning we error back on Next(). I'll fix in #646

[gingerwizard]

@ortyomka can you comment out the failing test - I think this is a separate issue. #646 contains the new decoder - I fixed the above issue. We can probably merge these in an order but this might be simpler to do first.

[ortyomka]

@gingerwizard one moment

[ortyomka]

@gingerwizard Done

[gingerwizard]

@ortyomka please resolve the conflicts now #646 was merged - should be easy but ping me if any issues.
On the whole Id only ask that you flatten the tests using require - the deep nesting is hard to the eye (its a pattern in the tests id like to remove). See the JSON tests https://github.com/ClickHouse/clickhouse-go/blob/v2/tests/json_test.go for cleaner examples.

The last unresolved point is temporary tables. I think for now we can rely on the user having to specify the context if they need consistent routing. I'm going to write the new doc soon - I'll make sure to note.

[gingerwizard]

Actually leave the test flattenning, ill do as a separate PR on all tests. Just resolve conflicts @ortyomka and LGTM.

[ortyomka]

@gingerwizard resolved

[gingerwizard]

Merging, thanks to @ortyomka and also for the patience.