Migrate CaseInsensitiveMapping tests to BaseCaseInsensitiveMappingTest by OLMS99 · Pull Request #8374 · trinodb/trino

OLMS99 · 2021-06-24T14:28:58Z

cla-bot · 2021-06-24T14:29:00Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

hashhar

Thanks for working on it - some comments.

Also can you please change the commit message to Migrate CaseInsensitiveMapping tests to BaseCaseInsensitiveMappingTest.

...rino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryCaseInsensitiveMapping.java

OLMS99 · 2021-06-24T15:29:26Z

Working on it and two questions: Is there a way to make the pull request without using my virtual machine's name and use my username instead? And Is there a way to edit the name out of cla-bot's message?

hashhar · 2021-06-24T16:02:44Z

Working on it and two questions: Is there a way to make the pull request without using my virtual machine's name and use my username instead? And Is there a way to edit the name out of cla-bot's message?

You can follow the instructions from the bot. Specifically git config --global user.email <your-email> and git config --global user.name <username or your name>.

cla-bot · 2021-06-24T16:49:53Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

OLMS99 · 2021-06-24T16:51:25Z

I followed the instructions and the username is still the machine's name, any idea?

hashhar

I can see the new commit has the correct identity OLMS99 <olavolucasprogrammer@protonmail.com>.
You might want to squash both commits together.

...rino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryCaseInsensitiveMapping.java

OLMS99 · 2021-06-24T17:24:04Z

And what about the clabot?

hashhar · 2021-06-24T17:28:36Z

And what about the clabot?

The CLA bot picks the name from the commit. Once you push all commits with new name it'll reflect there.
Also, have you already sent in the CLA via email?

cla-bot · 2021-06-24T17:32:56Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

OLMS99 · 2021-06-24T17:34:09Z

Yes, I've sent 3 days ago

hashhar · 2021-06-24T18:15:10Z

@OLMS99 The CLA is a manual process and it'll get approved in some time. Meanwhile can you fix the failing tests and the checkstyle errors? It might be easier to tackle this as 3 separate commits (one for each connector).

cla-bot · 2021-06-26T18:46:30Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-06-26T22:43:47Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-06-27T19:34:39Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-06-28T21:03:21Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-06-29T00:10:25Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

OLMS99 · 2021-06-29T20:44:47Z

@hashhar hey so, any idea what's causing error in pt (default, suite-7-non-generic)? And is there any reference to Trino coding conventions? Does the memsql.license on the remote repository in order?

hashhar · 2021-06-30T05:05:47Z

There was an issue with SQL Server docker images being moved. It's fixed now. See #8416

For codestyle follow the instructions at https://github.com/trinodb/trino/blob/master/DEVELOPMENT.md#code-style.

cla-bot · 2021-06-30T21:24:18Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-01T23:08:27Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

OLMS99 · 2021-07-03T14:17:41Z

@hashhar Working on the TestBigQueryCaseInsensitiveMapping, and there's a different Table and Schema naming convention (TestOracleCaseInsensitiveMapping have this same situation), should I ignore this in favor to BaseCaseInsensitiveMappingTest naming convention, the other way around or find a way to keep both?

cla-bot · 2021-07-06T23:02:22Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-07T13:24:10Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-07T16:28:10Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-07T16:58:54Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

OLMS99 · 2021-07-08T20:17:25Z

@hashhar Hey there, my errorprone plugin stopped working, I'm using comunity version of intellj and I can't test the code because of it, any sugestion to solve it?

cla-bot · 2021-07-09T01:31:18Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-09T01:38:30Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-23T19:10:51Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-24T17:44:24Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-25T19:43:36Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-25T22:25:28Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

kokosing · 2021-07-26T10:53:13Z

@cla-bot check

cla-bot · 2021-07-26T10:53:15Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2021-07-26T10:53:16Z

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

kokosing · 2021-07-26T10:55:58Z

@OLMS99 Can you please rebase and squash all commits. This should make it easier to review this PR and also make @cla-bot happy as it is now saying:

2021-07-26T10:53:14.783Z INFO Some commits from the following contributors are not signed with a valid email address: Presto_fdp.

cla-bot · 2021-07-26T12:55:46Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

OLMS99 · 2021-07-26T13:25:42Z

Trying to squash

cla-bot · 2021-07-26T19:15:07Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Presto_fdp.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

kokosing · 2021-07-27T11:54:41Z

Presto_fdp authored and OLMS99 committed 17 hours ago

There is some issue with your account in GitHub. Somehow github thinks you are not the author the code. Can you please make github to display OLMS99 as an author of the commit?

hashhar · 2021-07-27T11:56:18Z

I think rebase/squash don't change author unless --reset-author is also passed. You can see what git sees by running git log --format=fuller and look at Author and Comitter fields.

cla-bot · 2021-07-27T13:43:12Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: OLMS99.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email email@example.com
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

kokosing · 2021-07-30T08:28:57Z

🎉

kokosing

Looks good. Few minor comments.

kokosing · 2021-07-30T08:29:19Z

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseCaseInsensitiveMappingTest.java

kokosing · 2021-07-30T08:30:41Z

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleCaseInsensitiveMapping.java

    }

-    @Test
+    @Test @Override


Can you please add a comment why method is overriden?

Please put each annotation in separate line

kokosing · 2021-07-30T08:31:12Z

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleCaseInsensitiveMapping.java

    public void testTableNameClash()
            throws Exception
    {
-        String[] nameVariants = {"\"casesensitivename\"", "\"CaseSensitiveName\"", "\"CASESENSITIVENAME\""};


why quotes were removed?

the methods withTable and withSchema at BaseCaseInsensitiveappingTest.java insert the quotes on the names received using the method quoted.

hashhar

Looks mostly good % Oracle changes and @kokosing's comments.

I'll verify the MemSQL and BigQuery changes since they don't run on CI.
Thanks for consistently and patiently working on this @OLMS99.

hashhar · 2021-07-30T09:04:04Z

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlCaseInsensitiveMapping.java

+    protected SqlExecutor onRemoteDatabase()
    {
-        memSqlServer.execute(sql);
+        return memSqlServer::execute;


You can inline the function here. So something like return sql -> memSqlServer.execute(sql) and then remove the execute function since it's not used anywhere anymore.

ok, do the inline at onRemoteDataBase() of TestOracleCaseInsensitiveMapping.java too? @hashhar

OLMS99 · 2021-08-05T21:49:34Z

About the "user trino_test doesn't exist" problem, if I put a withSchema("trino_test"), it's raised an error of ambiguous username, and if I don't, it's raised an error of user "trino_user" doesn't exist. Any idea whats happening? @hashhar

OLMS99 · 2021-08-21T23:30:43Z

@hashhar ?

hashhar · 2022-03-15T07:27:12Z

Done in #11250

hashhar reviewed Jun 24, 2021

View reviewed changes

hashhar changed the title ~~attempt to solve issue #7864 Migrate CaseInsensitiveMapping tests to …~~ Migrate CaseInsensitiveMapping tests to BaseCaseInsensitiveMappingTest Jun 24, 2021

hashhar reviewed Jun 24, 2021

View reviewed changes

...rino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryCaseInsensitiveMapping.java Outdated Show resolved Hide resolved

OLMS99 force-pushed the trino_issue7864 branch from 0a5e641 to e297bd7 Compare July 26, 2021 12:55

OLMS99 force-pushed the trino_issue7864 branch from e297bd7 to 850389f Compare July 26, 2021 19:15

OLMS99 force-pushed the trino_issue7864 branch from 850389f to 2bbe015 Compare July 27, 2021 13:43

Migrate CaseInsensitiveMapping tests to BaseCaseInsensitiveMappingTest

d3b29d6

OLMS99 force-pushed the trino_issue7864 branch from 2bbe015 to d3b29d6 Compare July 27, 2021 15:02

cla-bot bot added the cla-signed label Jul 27, 2021

kokosing reviewed Jul 30, 2021

View reviewed changes

hashhar reviewed Jul 30, 2021

View reviewed changes

dealing with oracle test errors

ea6d89f

findepi force-pushed the master branch from 8538e49 to 1f896ea Compare July 30, 2021 22:13

Merge branch 'trinodb:master' into trino_issue7864

4f10146

hashhar closed this Mar 15, 2022

Conversation

OLMS99 commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cla-bot bot commented Jun 24, 2021

Uh oh!

hashhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OLMS99 commented Jun 24, 2021

Uh oh!

hashhar commented Jun 24, 2021

Uh oh!

cla-bot bot commented Jun 24, 2021

Uh oh!

OLMS99 commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hashhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

OLMS99 commented Jun 24, 2021

Uh oh!

hashhar commented Jun 24, 2021

Uh oh!

cla-bot bot commented Jun 24, 2021

Uh oh!

OLMS99 commented Jun 24, 2021

Uh oh!

hashhar commented Jun 24, 2021

Uh oh!

cla-bot bot commented Jun 26, 2021

Uh oh!

cla-bot bot commented Jun 26, 2021

Uh oh!

cla-bot bot commented Jun 27, 2021

Uh oh!

cla-bot bot commented Jun 28, 2021

Uh oh!

cla-bot bot commented Jun 29, 2021

Uh oh!

OLMS99 commented Jun 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hashhar commented Jun 30, 2021

Uh oh!

cla-bot bot commented Jun 30, 2021

Uh oh!

cla-bot bot commented Jul 1, 2021

Uh oh!

OLMS99 commented Jul 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cla-bot bot commented Jul 6, 2021

Uh oh!

cla-bot bot commented Jul 7, 2021

Uh oh!

cla-bot bot commented Jul 7, 2021

Uh oh!

cla-bot bot commented Jul 7, 2021

Uh oh!

OLMS99 commented Jul 8, 2021

Uh oh!

cla-bot bot commented Jul 9, 2021

Uh oh!

cla-bot bot commented Jul 9, 2021

Uh oh!

cla-bot bot commented Jul 23, 2021

Uh oh!

cla-bot bot commented Jul 24, 2021

Uh oh!

cla-bot bot commented Jul 25, 2021

Uh oh!

cla-bot bot commented Jul 25, 2021

Uh oh!

kokosing commented Jul 26, 2021

OLMS99 commented Jun 24, 2021 •

edited

Loading

OLMS99 commented Jun 24, 2021 •

edited

Loading

OLMS99 commented Jun 29, 2021 •

edited

Loading

OLMS99 commented Jul 3, 2021 •

edited

Loading

OLMS99 Jul 30, 2021 •

edited

Loading

OLMS99 commented Aug 5, 2021 •

edited

Loading