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

[feature-wip](arrow-flight)(step5) Support JDBC and PreparedStatement and Fix Bug #27661

Merged
merged 8 commits into from Nov 29, 2023

Conversation

xinyiZzz
Copy link
Contributor

@xinyiZzz xinyiZzz commented Nov 27, 2023

Proposed changes

Design Documentation Linked to #25514

  1. Support jdbc:arrow-flight-sql, which allows Java to connect to Doris Arrow Flight SQL Server using JDBC protocol.. see: https://www.dremio.com/drivers/jdbc/, https://arrow.apache.org/adbc/main/driver/jdbc.html, https://arrow.apache.org/cookbook/java/jdbc.html, https://arrow.apache.org/docs/java/flight_sql.html
  2. Supports PreparedStatement, but can only execute complete SQL, not support SQL parameters. Because JDBC can only use PreparedStatement to execute query, see: [Java] jdbc:arrow-flight-sql how to execute query without using PreparedStatement arrow#38742
  3. Temporarily upgrade Arrow to dev version 15.0.0-SNAPSHOT, because the latest release version Arrow 14.0.1 jdbc:arrow-flight-sql has BUG, see: [Java] jdbc:arrow-flight-sql executeQuery behaves incorrectly arrow#38785
  4. Support query results with anonymous columns
  5. Fix ResultSinkType error
  6. Support explain stmt
  7. Supports separately specifying BE’s Arrow Flight Server Host
  8. ...

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@xinyiZzz
Copy link
Contributor Author

run buildall

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
Tpch sf100 test result on commit 2986657a4b5878ee563c5cbb730aabe732d0f771, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4887	4653	4648	4648
q2	371	144	161	144
q3	2067	1896	1883	1883
q4	1385	1247	1243	1243
q5	3941	3937	3969	3937
q6	259	132	132	132
q7	1393	879	883	879
q8	2796	2808	2775	2775
q9	10207	10259	10288	10259
q10	3441	3540	3529	3529
q11	371	254	254	254
q12	442	298	304	298
q13	4564	3826	3836	3826
q14	324	284	284	284
q15	578	538	515	515
q16	662	579	579	579
q17	1148	974	935	935
q18	7910	7566	7420	7420
q19	1686	1695	1671	1671
q20	573	308	314	308
q21	4446	4029	3967	3967
q22	468	373	379	373
Total cold run time: 53919 ms
Total hot run time: 49859 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4595	4553	4566	4553
q2	357	240	269	240
q3	4046	4017	4014	4014
q4	2733	2716	2709	2709
q5	9713	9645	9677	9645
q6	242	124	125	124
q7	3051	2498	2517	2498
q8	4475	4445	4450	4445
q9	13304	13185	13148	13148
q10	4035	4169	4156	4156
q11	783	655	640	640
q12	993	801	815	801
q13	4289	3557	3614	3557
q14	370	345	343	343
q15	571	521	516	516
q16	745	672	698	672
q17	3880	3870	3871	3870
q18	9637	9138	9126	9126
q19	1795	1765	1771	1765
q20	2414	2038	2038	2038
q21	8762	8646	8768	8646
q22	872	789	830	789
Total cold run time: 81662 ms
Total hot run time: 78295 ms

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 46.54 seconds
stream load tsv: 568 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 28 seconds loaded 2358488459 Bytes, about 80 MB/s
stream load orc: 69 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.7 seconds inserted 10000000 Rows, about 348K ops/s
storage size: 17098784136 Bytes

be/src/common/config.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@xinyiZzz xinyiZzz force-pushed the 20231110_flight_add_regression_test branch from 6b37548 to 287056d Compare November 29, 2023 03:53
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@xinyiZzz
Copy link
Contributor Author

run buildall

@xinyiZzz
Copy link
Contributor Author

run buildall

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

@yiguolei yiguolei left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 29, 2023
Copy link
Contributor

PR approved by at least one committer and no changes requested.

Copy link
Contributor

PR approved by anyone and no changes requested.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@xinyiZzz
Copy link
Contributor Author

run buildall

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 43.75 seconds
stream load tsv: 566 seconds loaded 74807831229 Bytes, about 126 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 29.0 seconds inserted 10000000 Rows, about 344K ops/s
storage size: 17100899685 Bytes

Copy link
Contributor

@wangbo wangbo left a comment

Choose a reason for hiding this comment

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

LGTM

@yiguolei yiguolei merged commit d96e2df into apache:master Nov 29, 2023
25 checks passed
XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. meta-change reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants