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

support query distributed table with JSON column #956

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lrita
Copy link

@lrita lrita commented Apr 1, 2023

Summary

Support query distributed table with JSON column. When distributed table with JSON column, clickhouse not return Tuple column as query result. e.g. with table:

CREATE TABLE xxx.xx_log_cl
(
    `log_time` DateTime64(3, 'Asia/Shanghai'),
    `log_type` String,
    `ip` String,
    `client_ip` String,
    `id` String,
    `request_id` String,
    `category` String,
    `service_type` Int32,
    `logs` Object('json')
)
ENGINE = Distributed('cluster_1', 'xxx', 'xx_log', rand())

current code crash with:

panic: Not implemented [recovered]
	panic: Not implemented

goroutine 12 [running]:
testing.tRunner.func1.2({0xbdba20, 0xe90eb0})
	/Users/yanqing11/dev/go/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/Users/yanqing11/dev/go/src/testing/testing.go:1392 +0x39f
panic({0xbdba20, 0xe90eb0})
	/Users/yanqing11/dev/go/src/runtime/panic.go:838 +0x207
github.com/ClickHouse/clickhouse-go/v2/lib/column.(*JSONObject).ScanRow(0xc00070f8e8?, {0xbbedc0?, 0xc000236740?}, 0x40d7e5?)
	/Users/yanqing11/dev/app/src/github.com/ClickHouse/clickhouse-go/lib/column/json.go:856 +0x27
github.com/ClickHouse/clickhouse-go/v2.scan(0xc0000302c0, 0x1, {0xc000010120?, 0x9, 0xbbed01?})
	/Users/yanqing11/dev/app/src/github.com/ClickHouse/clickhouse-go/scan.go:82 +0x1ab
github.com/ClickHouse/clickhouse-go/v2.(*rows).Scan(0xbbedc0?, {0xc000010120?, 0x10?, 0xcebc7e?})
	/Users/yanqing11/dev/app/src/github.com/ClickHouse/clickhouse-go/clickhouse_rows.go:75 +0xa5
github.com/ClickHouse/clickhouse-go/v2/tests.TestMultipleJSONRows(0xc0006c29c0)
	/Users/yanqing11/dev/app/src/github.com/ClickHouse/clickhouse-go/tests/json_test.go:317 +0x222d
testing.tRunner(0xc0006c29c0, 0xda9400)
	/Users/yanqing11/dev/go/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/Users/yanqing11/dev/go/src/testing/testing.go:1486 +0x35f

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
    • I have no friendly way to create Distributed table in unit test with test docker container and extern clickhouse cluster. Then there is no uint test code commit.

@CLAassistant
Copy link

CLAassistant commented Apr 1, 2023

CLA assistant check
All committers have signed the CLA.

@jkaflik
Copy link
Contributor

jkaflik commented Apr 3, 2023

@lrita, thanks for your contribution. Can you please add test cases, at least with the happy path?

if jCol.decoding != nil {
return jCol.decoding.Row(i, ptr)
}
panic("bug")
Copy link
Contributor

Choose a reason for hiding this comment

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

what is exactly the case here? Decode func wasn't called prior?
let's make it more describtive

@jkaflik
Copy link
Contributor

jkaflik commented Apr 4, 2023

@lrita please have a look at a test checks report. (please skip cloud tests, since it's expected to break for ppl outside org)

@jkaflik jkaflik added the stale requires a follow-up label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale requires a follow-up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants