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.

Sign up for GitHub

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

Jump to bottom

[CALCITE-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type #3705

Merged
merged 1 commit into from
Mar 13, 2024
Merged

[CALCITE-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type #3705

merged 1 commit into from
Mar 13, 2024

Conversation

caicancai
Copy link
Member

@caicancai caicancai commented Feb 26, 2024 •
edited

@caicancai caicancai marked this pull request as draft February 26, 2024 15:15
@caicancai caicancai mentioned this pull request Feb 27, 2024
Merged
@caicancai
Copy link
Member Author

The code is very ugly, please do not review it, I will refactor it later.

@caicancai caicancai marked this pull request as ready for review February 29, 2024 03:08
@chucheng92
Copy link
Member

The code is very ugly, please do not review it, I will refactor it later.

you can create a DRAFT PR when PR is not ready.

@caicancai caicancai marked this pull request as draft February 29, 2024 03:14
@caicancai caicancai marked this pull request as ready for review February 29, 2024 03:32
@chucheng92
Copy link
Member

@caicancai A suggestion, don’t directly @/pin committers to review for you. This will affect other committers or contributors’ participation in your reviews.

@caicancai
Copy link
Member Author

@caicancai A suggestion, don’t directly @/pin committers to review for you. This will affect other committers or contributors’ participation in your reviews.

Thanks for your reminder

@caicancai caicancai force-pushed the 5976 branch 2 times, most recently from 451e2fb to cf664f8 Compare March 1, 2024 04:56
@chucheng92 chucheng92 changed the title [CALCITE-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [CALCITE-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type Mar 1, 2024
@chucheng92
Copy link
Member

@caicancai please use correct/proper commit name when you fix code-reivews to add a new commit.

@chucheng92
Copy link
Member

chucheng92 commented Mar 1, 2024 •
edited

@caicancai I have created a commit based on your PR. you can pick and take a look.

chucheng92@12cd99c

@caicancai
Copy link
Member Author

@caicancai I have created a commit based on your PR. you can pick and take a look. https://github.com/chucheng92/calcite/tree/CALCITE-5976

Thank you, very much appreciated

@caicancai caicancai force-pushed the 5976 branch 3 times, most recently from d0cbd5d to 754787c Compare March 5, 2024 00:11
@caicancai
Copy link
Member Author

caicancai commented Mar 5, 2024 •
edited

no comment. squashing

@chucheng92
Copy link
Member

@caicancai please change your commit/pr name.

@caicancai caicancai changed the title [CALCITE-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [CALCITE-5976] Function ArrayPrepend/ArrayAppend/ArrayInsert gives exception when inserted element type not equals array component type Mar 5, 2024
@caicancai
Copy link
Member Author

@caicancai please change your commit/pr name.

Thank you

@caicancai
Copy link
Member Author

@chucheng92 I've added you as Co-authored, thank you very much

@chucheng92
Copy link
Member

chucheng92 commented Mar 6, 2024 •
edited

@caicancai looks good to me now. could you change commit name by using 'ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT'. These function names are usually capitalized in commit.

@caicancai caicancai changed the title [CALCITE-5976] Function ArrayPrepend/ArrayAppend/ArrayInsert gives exception when inserted element type not equals array component type [CALCITE-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type Mar 6, 2024
@caicancai
Copy link
Member Author

@chucheng92 I fixed a similar problem. If you have time, can you help me take a look at this PR? Thank you.
#3721

@chucheng92
Copy link
Member

@chucheng92 I fixed a similar problem. If you have time, can you help me take a look at this PR? Thank you. #3721

looks good to me now, good job. @caicancai

@chucheng92 chucheng92 added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Mar 12, 2024
@caicancai @chucheng92
[CALCITE-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives…
9e72d0f 
… exception when inserted element type not equals array component type

Co-authored-by: Ran Tao <chucheng.tr@gmail.com>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 12, 2024

Quality Gate Passed Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
99.0% Coverage on New Code
56.1% Duplication on New Code

See analysis details on SonarCloud

@chucheng92
Copy link
Member

merging ...

@chucheng92 chucheng92 merged commit f0dc2b0 into apache:main Mar 13, 2024
17 checks passed
@caicancai caicancai deleted the 5976 branch March 13, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chucheng92 chucheng92 chucheng92 approved these changes

Assignees
No one assigned
Labels
LGTM-will-merge-soon Overall PR looks OK. Only minor things left.
Projects
None yet
Milestone
No milestone
2 participants
@caicancai @chucheng92