fivetran / dbt_jira

Data models for Fivetran's Jira connector built using dbt.
https://fivetran.github.io/dbt_jira/
Apache License 2.0
8 stars 15 forks source link

[Bug] Invalid column name in issue_field_history_columns #71

Closed crowemi closed 1 year ago

crowemi commented 1 year ago

Is there an existing issue for this?

Describe the issue

When using the issue_field_history_columns with a column having a leading numeric character in the name, the package fails. E.g. column name "1st reply date"

Relevant error log or model output

23:23:22  Database Error in model int_jira__pivot_daily_field_history (models/intermediate/field_history/int_jira__pivot_daily_field_history.sql)
23:23:22    001003 (42000): SQL compilation error:
23:23:22    syntax error line 29 at position 110 unexpected '1'.
23:23:22    compiled SQL at target/run/jira/models/intermediate/field_history/int_jira__pivot_daily_field_history.sql

Expected behavior

I expect the packages to succeed and create a column for the custom field in the jira__issues_enhanced and jira__daily_issue_field_history models with the corresponding values from the custom field.

dbt Project configurations

vars:
  # jira settings
  jira_database: <database>
  jira_schema: jira
  jira_using_sprints: false
  jira_using_components: false
  issue_field_history_columns: [
    '1st reply date'
  ]

Package versions

  - package: fivetran/jira
    version: 0.8.2
  - package: fivetran/jira_source
    version: 0.4.2

What database are you using dbt with?

snowflake

dbt Version

1.2.0

Additional Context

No response

Are you willing to open a PR to help address this issue?

crowemi commented 1 year ago

Proposed solution Slack discussion

fivetran-joemarkiewicz commented 1 year ago

Hi @crowemi thanks so much for sharing this detailed description of the bug you are experiencing. I am sorry to see you are experiencing it.

My one question is how this solution would impact the use of the data? My understanding is that the solution will remove the number from the name of the field. Would this cause confusion? For example, in your field shared above it would change from 1st reply date to st reply date. Is my understanding correct? I would just want to make sure that a solution we implement doesn't cause more confusion.

crowemi commented 1 year ago

it actually prefixes an '_' to the name, so it would be _1st reply date -- we could put this behind a flag so that the functionality is disabled by default. It seems like a one-off, but you could also add additional logic or dynamic regex to configs which might come in handy for someone down the line -- you never know what type of config you're gonna get on these free-form fields from third-party apps 😆🤷‍♂️

fivetran-joemarkiewicz commented 1 year ago

Thanks for the clarification @crowemi! With this, I actually think your suggestion may be a great addition to the package. Especially since we already handle a similar renaming with the slugify macro in this part of the package.

I noticed you are open to creating a PR. If you wanted to open one we can review in more detail and provide any additional notes in the process.

fivetran-catfritz commented 1 year ago

Hi @crowemi, to give you an update, our packages team is participating in a dbt hackathon this week and thought this could be a great fix to apply to a wider application outside of the jira packages. We would love to collaborate with you if you're on board! We were thinking our team would handle opening a PR against the slugify macro in dbt_utils and then tag you for feedback and as a contributor. Let me know what you think!

crowemi commented 1 year ago

sounds great - thank you, @fivetran-catfritz 😄

fivetran-catfritz commented 1 year ago

Thanks, @crowemi! We should be submitting this sometime this week!

fivetran-catfritz commented 1 year ago

@crowemi Wanted to provide an update here. I think you saw that this change was merged into the dbt_utils v1 branch, so when dbt releases it, that should effectively close out this issue! Thanks again for your collaboration on this!