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] Fix substr start index issue #434

Merged
merged 4 commits into from
Dec 20, 2023

Conversation

jerryleooo
Copy link
Contributor

@jerryleooo jerryleooo commented Jul 7, 2023

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

Fix #433

How was this patch tested?

Modify the existing unit tests accordingly.

@jerryleooo jerryleooo marked this pull request as draft July 7, 2023 10:30
@jerryleooo jerryleooo marked this pull request as ready for review July 8, 2023 09:45
@jerryleooo jerryleooo changed the title trying fix substr fix substr start index issue Jul 8, 2023
@jerryleooo
Copy link
Contributor Author

Hi @wmoustafa can help have a review? Thanks

@jerryleooo jerryleooo changed the title fix substr start index issue [Coral-Trino] fix substr start index issue Aug 1, 2023
@jerryleooo
Copy link
Contributor Author

@wmoustafa @findepi can have a look?

Copy link
Collaborator

@ljfgem ljfgem left a comment

Choose a reason for hiding this comment

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

Thank you @jerryleooo for this PR!

@ljfgem
Copy link
Collaborator

ljfgem commented Aug 2, 2023

And I think coral-dbt/src/main/resources/tests/__pycache__/test_incremental.cpython-311.pyc is not needed?

@jerryleooo
Copy link
Contributor Author

jerryleooo commented Aug 4, 2023

Thank @ljfgem for reviewing, I have fixed all the issues mentioned and added .pyc into .gitignore, so no .pyc file will be included in the commit.

@jerryleooo jerryleooo requested a review from ljfgem August 4, 2023 09:28
@ljfgem ljfgem changed the title [Coral-Trino] fix substr start index issue [Coral-Trino] Fix substr start index issue Aug 5, 2023
Copy link
Collaborator

@ljfgem ljfgem left a comment

Choose a reason for hiding this comment

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

LGTM

@ebyhr
Copy link
Contributor

ebyhr commented Dec 20, 2023

@ljfgem Gentle reminder.

@wmoustafa
Copy link
Contributor

@ljfgem is this pending anything or can we merge?

@ljfgem ljfgem merged commit a51a0db into linkedin:master Dec 20, 2023
1 check failed
@jerryleooo jerryleooo deleted the fix_substr_2 branch December 20, 2023 15:31
@ljfgem
Copy link
Collaborator

ljfgem commented Dec 22, 2023

@jerryleooo @ebyhr new version with the fix 2.2.23 has been released. Thanks for your contributions.

aastha25 added a commit to aastha25/coral that referenced this pull request Feb 23, 2024
aastha25 added a commit that referenced this pull request Feb 23, 2024
…492)

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

This reverts commit a51a0db.

* upgrade version to publish

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

Trino's substr has different start index
4 participants