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

File based column mapping for JDBC connectors #20413

Closed
wants to merge 2 commits into from

Conversation

IlyaEp
Copy link
Contributor

@IlyaEp IlyaEp commented Jan 18, 2024

Description

PR for issue #8745. Added the ability to specify mapping for columns in case-insensitive-name-matching.config-file file, like this:

{
   "schemas": [
      {
         "remoteSchema": "remote_schema",
         "mapping": "trino_schema"
      }],
   "tables": [
      {
         "remoteSchema": "remote_schema",
         "remoteTable": "remote_table",
         "mapping": "trino_table"
      }],
   "columns": [
      {
         "remoteSchema": "remote_schema",
         "remoteTable": "remote_table",
         "remoteColumn": "COL"
         "mapping": "trino_column1"
      },
      {
         "remoteSchema": "remote_schema",
         "remoteTable": "remote_table",
         "remoteColumn": "col"
         "mapping": "trino_column2"
     }]
}

The main idea of the implementation: add a new field to the JdbcColumnHandle class that stores name of a remote column and use value of this field instead of "Trino name" in the required places.

Additional context and related issues

Fixes #8745

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
(x) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

Copy link

cla-bot bot commented Jan 18, 2024

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

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

@cla-bot cla-bot bot added the cla-signed label Jan 18, 2024
@hashhar
Copy link
Member

hashhar commented Jan 19, 2024

The JDBC connector code is already hard to reason about with respect to remote vs local table and schema names.

I'd rather prefer effort being spent on fixing the root cause (#17) instead of introducing additional complexity.

@hashhar
Copy link
Member

hashhar commented Jan 19, 2024

I'll defer to other maintainers on their opinion about this : @trinodb/maintainers

@IlyaEp
Copy link
Contributor Author

IlyaEp commented Jan 29, 2024

@trinodb/maintainers Please take a look

@mosabua
Copy link
Member

mosabua commented Jan 29, 2024

I tend to agree with @hashhar that this is a complex workaround with a bunch of configuration and it avoids fixing the underlying issue. We will discuss to see if we want to support this approach.

Also @IlyaEp .. if you want to fix this for yourself you could just create views in the underlying database that use the same data and change the column names to be lowercase and then you can query it all in Trino without problems.

@findepi
Copy link
Member

findepi commented Jan 30, 2024

I'd rather prefer effort being spent on fixing the root cause (#17) instead of introducing additional complexity.

I like the idea especially that, from what i know, fixing #17 is supposed to happen.

Copy link

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Feb 20, 2024
Copy link

Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time.

@github-actions github-actions bot closed this Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

File based column mapping for JDBC connectors
4 participants