[trino-ignite] Ensure primary_key columns exist while creating table #16266

sahoss · 2023-02-25T04:57:56Z

Description

Adds a check to validate that the column names defined in primary_key table property exists as column names in the table definition.

Fixes #16215

Release notes

(x) This is not user-visible or docs only and no release notes are required.

cla-bot · 2023-02-25T04:57:58Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteClient.java

cla-bot · 2023-02-25T05:01:34Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

sahoss · 2023-02-25T05:02:34Z

@chenjian2664 @ebyhr please take a look :)

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteClient.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

cla-bot · 2023-02-25T12:04:55Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2023-02-25T12:15:55Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

cla-bot · 2023-02-25T12:20:15Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

ebyhr

Thanks for your contribution. Please squash commits into one and follow this guideline for commit messages. https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#format-git-commit-messages

By the way, have you already submit CLA?

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

sahoss · 2023-02-26T08:29:47Z

Thanks for your contribution. Please squash commits into one and follow this guideline for commit messages. https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#format-git-commit-messages

:)

Will do this

By the way, have you already submit CLA?

Yes. i have submit it via email, but yet to be accepted.

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

ebyhr · 2023-02-26T23:09:21Z

@sahoss Please feel free to ping me after addressing comments.

cla-bot · 2023-02-27T16:05:17Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

sahoss · 2023-02-27T16:44:24Z

@sahoss Please feel free to ping me after addressing comments.

@ebyhr Done. please take a look.
328e5a6 deletes the unused branch.

#16266 (comment) and #16266 (comment) is addressed as well.

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

cla-bot · 2023-02-28T08:22:53Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2023-02-28T13:51:28Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2023-03-01T07:00:20Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2023-03-01T10:58:53Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

sahoss · 2023-03-03T06:45:16Z

@ebyhr wondering if you could take a look at this pr (#16266 (comment) has been addressed)

also, verification/cla-signed bot is failing, but i have mailed my CLA. anything/anyone specifically i should do or reachout to? thanks.

ebyhr · 2023-03-07T22:56:30Z

@cla-bot check

cla-bot · 2023-03-07T22:56:34Z

The cla-bot has been summoned, and re-checked this pull request!

ebyhr

Remove unreachable branch and for loop

I would change to "Remove unreachable code from IgniteClient"

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

sahoss · 2023-03-11T13:36:32Z

@ebyhr updated the commit message and updated the test_name variable. ptal!

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

Exception was unreachable as the same check is also present from create table task

Adds a check for create table in IgniteClient. The check ensures that the column names mentioned in the table property 'primary_key' exists in the column defined in the create table statement.

sahoss commented Feb 25, 2023

View reviewed changes

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteClient.java Outdated Show resolved Hide resolved

sahoss marked this pull request as ready for review February 25, 2023 04:59

chenjian2664 reviewed Feb 25, 2023

View reviewed changes

sahoss commented Feb 25, 2023

View reviewed changes

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java Outdated Show resolved Hide resolved

sahoss requested a review from chenjian2664 February 25, 2023 12:20

ebyhr reviewed Feb 25, 2023

View reviewed changes

sahoss commented Feb 26, 2023

View reviewed changes

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java Show resolved Hide resolved

chenjian2664 approved these changes Feb 27, 2023

View reviewed changes

trinodb deleted a comment from cla-bot bot Feb 27, 2023

ebyhr reviewed Feb 27, 2023

View reviewed changes

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java Show resolved Hide resolved

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java Show resolved Hide resolved

cla-bot bot added the cla-signed label Mar 7, 2023

ebyhr approved these changes Mar 7, 2023

View reviewed changes

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java Show resolved Hide resolved

ebyhr reviewed Mar 13, 2023

View reviewed changes

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java Show resolved Hide resolved

kokosing force-pushed the master branch from 3f05134 to 58d6356 Compare March 14, 2023 11:34

sahoss added 2 commits March 15, 2023 11:25

Remove unreachable code from IgniteClient

1d1f2fa

Exception was unreachable as the same check is also present from create table task

Check primary_key exist during Ignite create table

ab3f364

Adds a check for create table in IgniteClient. The check ensures that the column names mentioned in the table property 'primary_key' exists in the column defined in the create table statement.

ebyhr merged commit 8df3c9c into trinodb:master Mar 20, 2023

github-actions bot added this to the 411 milestone Mar 20, 2023

colebow mentioned this pull request Mar 29, 2023

Add Trino 411 release notes #16552

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[trino-ignite] Ensure primary_key columns exist while creating table #16266

[trino-ignite] Ensure primary_key columns exist while creating table #16266

sahoss commented Feb 25, 2023 •

edited by ebyhr

Loading

cla-bot bot commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

sahoss commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

ebyhr left a comment

sahoss commented Feb 26, 2023 •

edited

Loading

ebyhr commented Feb 26, 2023

cla-bot bot commented Feb 27, 2023

sahoss commented Feb 27, 2023

cla-bot bot commented Feb 28, 2023

cla-bot bot commented Feb 28, 2023

cla-bot bot commented Mar 1, 2023

cla-bot bot commented Mar 1, 2023

sahoss commented Mar 3, 2023

ebyhr commented Mar 7, 2023

cla-bot bot commented Mar 7, 2023

ebyhr left a comment

sahoss commented Mar 11, 2023

[trino-ignite] Ensure primary_key columns exist while creating table #16266

[trino-ignite] Ensure primary_key columns exist while creating table #16266

Conversation

sahoss commented Feb 25, 2023 • edited by ebyhr Loading

Description

Release notes

cla-bot bot commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

sahoss commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

cla-bot bot commented Feb 25, 2023

ebyhr left a comment

Choose a reason for hiding this comment

sahoss commented Feb 26, 2023 • edited Loading

ebyhr commented Feb 26, 2023

cla-bot bot commented Feb 27, 2023

sahoss commented Feb 27, 2023

cla-bot bot commented Feb 28, 2023

cla-bot bot commented Feb 28, 2023

cla-bot bot commented Mar 1, 2023

cla-bot bot commented Mar 1, 2023

sahoss commented Mar 3, 2023

ebyhr commented Mar 7, 2023

cla-bot bot commented Mar 7, 2023

ebyhr left a comment

Choose a reason for hiding this comment

sahoss commented Mar 11, 2023

sahoss commented Feb 25, 2023 •

edited by ebyhr

Loading

sahoss commented Feb 26, 2023 •

edited

Loading