[sql] Upgrade PrestoSQL to the first Trino version #16683

tisonkun · 2022-07-19T14:02:13Z

Motivation

See also previous discussion at https://lists.apache.org/thread/mflm0pb5235jjk80vol0vs7v0hvowkq8.

Instead of firstly create a pulsar-sql repository and highly possibly leave it uncompleted, the original pain points of maintaining Pulsar SQL (a.k.a. Pulsar Presto Connector) are:

Pulsar (All) docker image is too large.
PrestoSQL version is too old and pollutes old dependencies.

Thus, I'd suggest we continuously kaizen the situation by upgrading version and decouple modules. When moving out the codebase to another repository a trivial step, we consider to do it.

This patch is a small step to upgrade PrestoSQL to the first Trino version which supports Apple M1, and it contains most of the renaming work.

After this change, if users running sql worker with pulsar provided scripts, the experience should be as usual. But people directly depend on pulsar-presto-connector will find the dependency is switched to Trino. I'm unsure whether or not I should write a PIP of this kaizen way.

I'll treat this change at the earliest in 2.12 release train and don't rush things.

Modifications

Update dependencies in pom.xml from io.prestosql to io.trino.
Update prestosql in code to trino.

~~Fortunately, the breaking changes mentioned in #16494 aren't included in this version bump.~~

Fix timestamp breaking changes in trinodb/trino@21e3ddf and trinodb/trino@19811d3. This exists in the first Trino version (351).
Update joda-time to 2.10.10 to catch up Trino 363's requirement, see 848e66f in details.

Verifying this change

Make sure that the change passes the CI checks.

This change is already covered by existing tests. We should keep all existing test passed.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (yes)

Documentation

doc-required
(Your PR needs to update docs and you will update later)
doc-not-needed

It should be required. But leave it as is to verify CI first.

Signed-off-by: tison <wander4096@gmail.com>

Otherwise trino-testing will throw Exception before running test cases. Signed-off-by: tison <wander4096@gmail.com>

See also: * trinodb/trino@21e3ddf * trinodb/trino@19811d3 Signed-off-by: tison <wander4096@gmail.com>

Signed-off-by: tison <wander4096@gmail.com>

Get America/Nuuk alike timezone. This version is required by trino. See also https://twitter.com/raphaelschaad/status/1488584437197586432. Signed-off-by: tison <wander4096@gmail.com>

Signed-off-by: tison <wander4096@gmail.com>

See also: * trinodb/trino@aaaa30d * trinodb/trino@8943325 Signed-off-by: tison <wander4096@gmail.com>

tisonkun · 2022-07-21T03:32:32Z

This patch should be technically completed.

cc @merlimat @lhotari @MarvinCai if you have spare time, please read the description above and see what direction is good to go. Or if we should go back to the mailing list to continue the discussion.

tisonkun · 2022-07-21T03:34:30Z

/pulsarbot run-failure-checks

tisonkun · 2022-07-21T05:55:11Z

/pulsarbot run-failure-checks

tisonkun · 2022-07-23T02:27:40Z

If we can upgrade prestosql to trino, we may have an opportunity to move conf/presto to trino/conf and lib/presto/* to trino/*, so that we can later smoothly move out the Pulsar SQL components.

merlimat

👍 Great work!

Signed-off-by: tison <wander4096@gmail.com>

tisonkun · 2022-07-25T14:14:28Z

Thank @merlimat for review! I push a new commit to fix license issues. Our CI doesn't check binary licenses of Pulsar SQL, though. It can be a separated issue to resolve and here is #16783.

cc @MarvinCai @lhotari @codelipenghui perhaps you can also give a review on this patch and decide how to move forward?

tisonkun · 2022-07-26T01:59:58Z

/pulsarbot run-failure-checks

tisonkun · 2022-07-26T05:05:50Z

/pulsarbot run-failure-checks

tisonkun · 2022-07-26T05:06:03Z

I think all required tasks have passed.

Signed-off-by: tison <wander4096@gmail.com>

tisonkun · 2022-07-28T01:07:19Z

/pulsarbot run-failure-checks

tisonkun · 2022-07-28T01:30:29Z

/pulsarbot run-failure-checks

tisonkun added 3 commits July 19, 2022 21:05

pom.xml prestosql to trino

98addfd

Signed-off-by: tison <wander4096@gmail.com>

code prestosql to trino

b7e90e1

Signed-off-by: tison <wander4096@gmail.com>

PrestoServer to TrinoServer

6254e0f

Signed-off-by: tison <wander4096@gmail.com>

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 19, 2022

tisonkun added 6 commits July 19, 2022 23:40

misc prestosql to trino

69e804f

Signed-off-by: tison <wander4096@gmail.com>

move to the first version supporting Apple M1

711b739

Signed-off-by: tison <wander4096@gmail.com>

jdbc presto to trino

26c1b4f

Signed-off-by: tison <wander4096@gmail.com>

follow trino testing conventions

aeabb9f

Otherwise trino-testing will throw Exception before running test cases. Signed-off-by: tison <wander4096@gmail.com>

fix timestamp payload

c418395

See also: * trinodb/trino@21e3ddf * trinodb/trino@19811d3 Signed-off-by: tison <wander4096@gmail.com>

fix pushdown timestamp payload

2605a5f

Signed-off-by: tison <wander4096@gmail.com>

tisonkun mentioned this pull request Jul 20, 2022

Add support for date, time_millis, and timestamp_millis to AvroColumnDecoder trinodb/trino#13070

Closed

Update joda-time to 2.10.0

848e66f

Get America/Nuuk alike timezone. This version is required by trino. See also https://twitter.com/raphaelschaad/status/1488584437197586432. Signed-off-by: tison <wander4096@gmail.com>

tisonkun force-pushed the prestosql-to-trino branch from db164cc to 848e66f Compare July 21, 2022 00:31

tisonkun added 2 commits July 21, 2022 09:30

fix pulsar-sql tests can crash after paased if run in parallel

eec2a50

Signed-off-by: tison <wander4096@gmail.com>

fix HTTP request to trino worker requires X-Trino-User header

dd7dc7a

See also: * trinodb/trino@aaaa30d * trinodb/trino@8943325 Signed-off-by: tison <wander4096@gmail.com>

tisonkun marked this pull request as ready for review July 21, 2022 03:30

merlimat approved these changes Jul 23, 2022

View reviewed changes

merlimat added this to the 2.11.0 milestone Jul 23, 2022

tisonkun added 2 commits July 25, 2022 17:50

Merge branch 'master' into prestosql-to-trino

8379689

fix LICENSE

65ad6d4

Signed-off-by: tison <wander4096@gmail.com>

tisonkun mentioned this pull request Jul 25, 2022

check-binary-license.sh doesn't check Pulsar SQL licenses in CI #16783

Closed

Merge branch 'master' into prestosql-to-trino

11d9987

sijie mentioned this pull request Jul 25, 2022

ISSUE-16783: check-binary-license.sh doesn't check Pulsar SQL licenses in CI streamnative/pulsar-archived#4615

Open

This comment was marked as outdated.

# to view

codelipenghui removed this from the 2.11.0 milestone Jul 26, 2022

codelipenghui added this to the 2.12.0 milestone Jul 26, 2022

tisonkun added 4 commits July 27, 2022 17:14

properly fix all time/timestamp payload

11d3b9f

Signed-off-by: tison <wander4096@gmail.com>

add and adjust tests for time/timestamp payload changes

5eaf28a

Signed-off-by: tison <wander4096@gmail.com>

Merge branch 'master' into prestosql-to-trino

05f7724

fix checkstyle

cc71f96

Signed-off-by: tison <wander4096@gmail.com>

codelipenghui approved these changes Aug 8, 2022

View reviewed changes

codelipenghui merged commit 47b6611 into apache:master Aug 8, 2022

codelipenghui assigned tisonkun Aug 8, 2022

codelipenghui added the area/sql Pulsar SQL related features label Aug 8, 2022

Technoboy- pushed a commit to merlimat/pulsar that referenced this pull request Aug 16, 2022

[sql] Upgrade PrestoSQL to the first Trino version (apache#16683)

c83a595

tisonkun mentioned this pull request Aug 17, 2022

PIP-200: Package Pulsar Trino distro and config in a dedicated folder #17137

Closed

1 task

sijie mentioned this pull request Aug 17, 2022

ISSUE-17137: PIP-200: Package Pulsar Trino distro and config in a dedicated folder streamnative/pulsar-archived#4749

Open

tisonkun deleted the prestosql-to-trino branch August 22, 2022 09:10

Jason918 mentioned this pull request Sep 22, 2022

[fix][ci][branch-2.10] Fix owasp ci failure on branch-2.10 #17765

Merged

13 tasks

This was referenced Dec 7, 2022

Trino #10105

Closed

[Issue 8091][Pulsar SQL] Add presto password authenticators plugin #9411

Closed

tisonkun mentioned this pull request Apr 5, 2023

[feat][sql] Bump Trino version to 368 and fix Decimal breaking change #20016

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sql] Upgrade PrestoSQL to the first Trino version #16683

[sql] Upgrade PrestoSQL to the first Trino version #16683

tisonkun commented Jul 19, 2022 •

edited

Loading

tisonkun commented Jul 21, 2022 •

edited

Loading

tisonkun commented Jul 21, 2022

tisonkun commented Jul 21, 2022

tisonkun commented Jul 23, 2022 •

edited

Loading

merlimat left a comment

tisonkun commented Jul 25, 2022

This comment was marked as outdated.

tisonkun commented Jul 26, 2022

tisonkun commented Jul 26, 2022

tisonkun commented Jul 26, 2022

tisonkun commented Jul 28, 2022

tisonkun commented Jul 28, 2022

[sql] Upgrade PrestoSQL to the first Trino version #16683

[sql] Upgrade PrestoSQL to the first Trino version #16683

Conversation

tisonkun commented Jul 19, 2022 • edited Loading

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

tisonkun commented Jul 21, 2022 • edited Loading

tisonkun commented Jul 21, 2022

tisonkun commented Jul 21, 2022

tisonkun commented Jul 23, 2022 • edited Loading

merlimat left a comment

Choose a reason for hiding this comment

tisonkun commented Jul 25, 2022

This comment was marked as outdated.

tisonkun commented Jul 26, 2022

tisonkun commented Jul 26, 2022

tisonkun commented Jul 26, 2022

tisonkun commented Jul 28, 2022

tisonkun commented Jul 28, 2022

tisonkun commented Jul 19, 2022 •

edited

Loading

tisonkun commented Jul 21, 2022 •

edited

Loading

tisonkun commented Jul 23, 2022 •

edited

Loading