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

Add support for Trino #376

Merged
merged 2 commits into from
Sep 20, 2023
Merged

Conversation

damian3031
Copy link
Contributor

This is a:

  • bug fix PR with no breaking changes
  • new functionality

Link to Issue

Closes #375

Checklist

  • I have verified that these changes work locally on the following warehouses (Note: it's okay if you do not have access to all warehouses, this helps us understand what has been covered)
    • BigQuery
    • Postgres
    • Redshift
    • Snowflake
    • Databricks
    • DuckDB
    • Trino
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)

@damian3031
Copy link
Contributor Author

I've added integration tests to CircleCI. Please add below environment variables relevant for your Starburst Galaxy instance (AFAIK you have one) so that the tests can run on it.
TRINO_TEST_HOST, TRINO_TEST_USER, TRINO_TEST_PASS, TRINO_TEST_PORT, TRINO_TEST_CATALOG_NAME

@b-per
Copy link
Collaborator

b-per commented Sep 15, 2023

Thanks @damian3031 !
I am testing it locally right now and we will add the Starburst credentials for CI/CD!

Based on my initial testing (using a Snowflake catalog) there are some issues (which might required updates to dbt-trino):

  • if a table has a description, it is added to the DDL but I then get the error TrinoUserError(type=USER_ERROR, name=NOT_SUPPORTED, message="This connector does not support creating tables with table comment", query_id=abcd)
    • is it possible to disable the comment part of the DDL? it doesn't seem to be based on the code I saw in dbt-trino
  • I am also getting some errors This connector does not support creating views which seems consistent with this issue.

Based on the observations above:

  • what catalog type would you recommend to avoid those errors? (or which one did you test with for now)
  • should we test the package for different catalog types?
  • we might need to add some info in the README to mention that some catalog types will work with Trino but not all

@damian3031
Copy link
Contributor Author

@b-per creating views is supported in object storage connectors such as Hive, Delta Lake, and Apache Iceberg.

I tested it on the Iceberg connector, and the tests passed.

I think it would be fair to test it specifically against Iceberg connector and mention it in the README.

README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@b-per b-per left a comment

Choose a reason for hiding this comment

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

Thanks for the work here!

All good with me with the small exception of updating the CI config file to run Trino at the same time as Snowflake/BQ etc...

I have added the env vars to test on S3 and it works

.circleci/config.yml Outdated Show resolved Hide resolved
@damian3031 damian3031 force-pushed the add-support-for-trino branch from fb3d81b to 4950ce7 Compare September 18, 2023 17:40
@b-per b-per merged commit 1fbc682 into dbt-labs:main Sep 20, 2023
@b-per
Copy link
Collaborator

b-per commented Sep 20, 2023

We'll do a new release of the package on the dbt Hub in just a beat so that it is accessible to users.
EDIT: this should read "bit" and not "beat", but "beat" kinda sounds good as well!

@andrea-montes-yello
Copy link

We'll do a new release of the package on the dbt Hub in just a beat so that it is accessible to users.

Yeiii, thank you!!

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.

Add support for Trino
3 participants