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 ability to configure index names #8656

Closed
wants to merge 1 commit into from

Conversation

dbeatty10
Copy link
Contributor

@dbeatty10 dbeatty10 commented Sep 16, 2023

resolves dbt-labs/dbt-postgres#52

This is a proof-of-concept to implement the feature request in dbt-labs/dbt-postgres#52.

I don't personally plan to take this one over the finish line, so will rely on @pscanlon1 or someone else to create their own PR using this as a guide.

TODO

Along with a PR, make sure that this documentation is updated to add the index name config:
https://docs.getdbt.com/reference/resource-configs/postgres-configs#indexes

Test case 1

models/my_model_index_name.sql

{{ config(
    materialized = 'table',
    indexes=[
      {'columns': ['column_a'], 'type': 'hash', 'name': 'this_is_my_index_name'},
    ]
)}}

select 1 as column_a, 2 as column_b
dbt run -s my_model_index_name

Test case 2

See #8655

models/my_mv_index_name.sql

{{ config(
    materialized="materialized_view",
    indexes=[
      {'columns': ['column_a'], 'type': 'hash', 'name': 'this_is_my_mv_index_name'},
    ],
    on_configuration_change="apply",
)}}

select 1 as column_a, 2 as column_b
dbt run -s my_mv_index_name

Now alter the columns definition:

models/my_mv_index_name.sql

{{ config(
    materialized="materialized_view",
    indexes=[
      {'columns': ['column_a'], 'type': 'hash', 'name': 'this_is_my_mv_index_name'},
    ],
    on_configuration_change="apply",
)}}

select 1 as column_a, 2 as column_b

And re-run it again and make sure the index was altered appropriately:

dbt run -s my_mv_index_name

Problem

Solution

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@dbeatty10 dbeatty10 added the user docs [docs.getdbt.com] Needs better documentation label Sep 16, 2023
@cla-bot cla-bot bot added the cla:yes label Sep 16, 2023
@github-actions
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.21% 🎉

Comparison is base (07372db) 86.34% compared to head (491e180) 86.55%.
Report is 79 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    dbt-labs/dbt-core#8656      +/-   ##
==========================================
+ Coverage   86.34%   86.55%   +0.21%     
==========================================
  Files         174      175       +1     
  Lines       25579    25638      +59     
==========================================
+ Hits        22087    22192     +105     
+ Misses       3492     3446      -46     
Flag Coverage Δ
integration 83.34% <ø> (+0.21%) ⬆️
unit 65.10% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 46 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lbelyaev
Copy link

hey @dbeatty10 , we've got to use dbt pretty recently and totally love it!. This issue a hurdle to enable smooth integration with Hasura GraphQL engine for our new API. The changes look pretty small, but the impact shall be huge. Any chance you need any help to move this forward? Our team would be ready to help if we can.

@dbeatty10
Copy link
Contributor Author

hey @dbeatty10 , we've got to use dbt pretty recently and totally love it!

Glad to hear @lbelyaev! 🤩

This issue a hurdle to enable smooth integration with Hasura GraphQL engine for our new API. The changes look pretty small, but the impact shall be huge. Any chance you need any help to move this forward? Our team would be ready to help if we can.

To move this forward, do the following:

  1. Open your own PR using this as a guide. The relevant code has moved to a new repo: https://github.com/dbt-labs/dbt-postgres. So just copy-paste the changes from this PR into your feature branch in dbt-postgres.
  2. Add tests to your PR. See the description within this PR for a couple relevant test cases.

Since this PR is no longer mergeable due to the the code living in dbt-postgres now, I'm going to close this PR.

But it would be awesome if you or someone else chooses to pick it up and run with it! 🏃

@dbeatty10 dbeatty10 closed this Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes user docs [docs.getdbt.com] Needs better documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-3086] [Feature] Add "name" to Postgres index configuration
2 participants