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

[Coral-Trino] Revert PR#434 - Do not increment substr starting index #492

Merged
merged 3 commits into from
Feb 23, 2024

Conversation

aastha25
Copy link
Contributor

Previously a PR - PR#434 added a transformation in Coral-Trino to always increment the starting index for 'substr' operator by 1 based on the issue filed #433 which assumed that hive support 0 based indexing for this operator & Trino only supports 1 based indexing.

However, based on recent explorations, it seems that Trino does supports only 1 based indexing. However, hive also supports 1 based indexing and considers 0 to be 1:

hive > create view test_substr0 as select substring('123', 0, 2) as a from t1;
hive > create view test_substr1 as select substring('123', 1, 2) as a from t1;

hive > select * from test_substr0;
OK
12
Time taken: 0.245 seconds, Fetched: 1 row(s)
hive > select * from test_substr1;
OK
12
Time taken: 0.191 seconds, Fetched: 1 row(s)

Hence, PR#434 needs to be reverted. A follow up PR will modify the logic of PR#434 to only update the index when the index is 0.

What changes are proposed in this pull request, and why are they necessary?

How was this patch tested?

./gradlew build

@aastha25 aastha25 merged commit b21af21 into linkedin:li-trino-hotfix Feb 23, 2024
1 check passed
KevinGe00 pushed a commit to KevinGe00/coral that referenced this pull request Feb 26, 2024
…inkedin#492)

* Revert "[Coral-Trino] Fix substr start index issue (linkedin#434)"

This reverts commit a51a0db.

* upgrade version to publish

* fix tests and stylecheck
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.

1 participant