fivetran / dbt_sage_intacct

Fivetran data models for Sage Intacct using dbt.
https://fivetran.github.io/dbt_sage_intacct/#!/overview
Apache License 2.0
5 stars 6 forks source link

[Bug] AP AR Enhanced Table - Numeric value '' Is Not Recognized #20

Closed matt-tkachev closed 11 months ago

matt-tkachev commented 11 months ago

Is there an existing issue for this?

Describe the issue

I've already opened a ticket with Fivetran Support, and they've asked me to report the bug here as well. So, I'm pasting the ticket text below. We've recently added a dbt transformation package for Sage Intacct in Fivetran, and all individual table transformations have worked, except for one. There is a "sage_intacct__ap_ar_enhanced" transformation that has failed with an error message: "Numeric value '' is not recognized". The error message makes me think that there is a varchar column that has an empty string entry ('') that the Fivetran transformation code is trying to convert to the numeric data type. However, the error message is very short and not insightful enough to realize what column causes this error.

Relevant error log or model output

Database Error in model sage_intacct__ap_ar_enhanced (models/sage_intacct__ap_ar_enhanced.sql)
100038 (22018): Numeric value '' is not recognized
compiled Code at target/run/sage_intacct/models/sage_intacct__ap_ar_enhanced.sql

Expected behavior

"SAGE__INTACCT_AP_AR_ENHANCED" table constructed and inserted into in our destination of Snowflake.

dbt Project configurations

config-version: 2
name: 'sage_intacct'
version: '0.2.2'
require-dbt-version: [">=1.3.0", "<2.0.0"]
models:
  sage_intacct:
    +materialized: table
    +schema: sage
    intermediate:
      +materialized: view
vars:
  sage_intacct_database: havenpark
  sage_intacct_schema: sage
  sage_account_pass_through_columns: []
  sage_gl_pass_through_columns: []
  sage_intacct:
    gl_detail: "{{ ref('stg_sage_intacct__gl_detail') }}"
    gl_account: "{{ ref('stg_sage_intacct__gl_account') }}"
    ap_bill_item: "{{ ref('stg_sage_intacct__ap_bill_item') }}"
    ap_bill: "{{ ref('stg_sage_intacct__ap_bill') }}"
    ar_invoice_item: "{{ ref('stg_sage_intacct__ar_invoice_item') }}"
    ar_invoice: "{{ ref('stg_sage_intacct__ar_invoice') }}"
    sage_gl_pass_through_columns: ['location_id', 'location_name']
    sage_account_pass_through_columns: []
    sage_intacct_category_asset: ('Inventory','Fixed Assets','Other Current Assets','Cash and Cash Equivalents','Intercompany Receivable','Accounts Receivable','Deposits and Prepayments','Goodwill','Intangible Assets','Short-Term Investments','Inventory','Accumulated Depreciation','Other Assets','Unrealized Currency Gain/Loss','Patents','Investment in Subsidiary','Escrows and Reserves','Long Term Investments')
    sage_intacct_category_equity: ('Partners Equity','Retained Earnings','Dividend Paid')
    sage_intacct_category_expense: ('Advertising and Promotion Expense','Other Operating Expense','Cost of Sales Revenue', 'Professional Services Expense','Cost of Services Revenue','Payroll Expense','Payroll Taxes','Travel Expense','Cost of Goods Sold','Other Expenses','Compensation Expense','Federal Tax','Depreciation Expense')
    sage_intacct_category_liability: ('Accounts Payable','Other Current Liabilities','Accrued Liabilities','Note Payable - Current','Deferred Taxes Liabilities - Long Term','Note Payable - Long Term','Other Liabilities','Deferred Revenue - Current')
    sage_intacct_category_revenue: ('Revenue','Revenue - Sales','Dividend Income','Revenue - Other','Other Income','Revenue - Services','Revenue - Products')
    sage_intacct__using_invoices: true
    sage_intacct__using_bills: true
analysis-paths: ["analysis"]

Package versions

packages:

What database are you using dbt with?

snowflake

dbt Version

dbtVersion: 1.3.0 jobs:

Additional Context

No response

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

fivetran-joemarkiewicz commented 11 months ago

Hi @matt-tkachev thanks for opening this issue. I wasn't able to reproduce this myself, but I am wondering how you are running the Sage Intacct package? It looks like the changes you have applied to the package are directly within the package's dbt_project.yml is this true?

You should be making any variable changes that you want the package to apply to be within your root dbt_project.yml. I have seen some instances where updating the package dbt_project.yml directly can result in some unexpected behavior, like what you may be seeing.

Additionally, can you inspect the target/run/sage_intacct/models/sage_intacct__ap_ar_enhanced.sql file of your project and share the compiled code? This should give some insight more directly into what is erroring out.

Let me know!

matt-tkachev commented 11 months ago

Hello @fivetran-joemarkiewicz we're running the Sage Intacct package as a Fivetran transformation where I've added my GitHub fork of the package repository. The only changes I've made to the dbt_project.yml file are related to the schema name since the package assumes the schema to be Sage Intacct while in our case it's just Sage; thus, I don't think that it should cause any functional difference.

Also, I believe it's important to mention that I've integrated the same package fork (without the schema name changes mentioned above since their schema is Sage Intacct as expected by the package) for a different client, and it's been working without any issues for all tables including AP AR Enhanced. So, I think that the issue may have appeared due to a difference in the Sage Intacct data that the code doesn't expect. Because again, the error message implies that there is a varchar column instead of a numeric one that the package code expects.

I'm attaching the target/run/sage_intacct/models/sage_intacct__ap_ar_enhanced.sql file contents below. As with regards to the compiled code, I'm not sure if I'm able to access it since it's run by Fivetran. Should I run it in another environment to get the compiled version?

{{ config(enabled=fivetran_utils.enabled_vars_one_true(vars=["sage_intacct__using_bills", "sage_intacct__using_invoices"])) }}

with

{% if var('sage_intacct__using_bills', True) %}
ap_bill as (
    select * 
    from {{ ref('stg_sage_intacct__ap_bill') }} 
), 

ap_bill_item as (
    select * 
    from {{ ref('stg_sage_intacct__ap_bill_item') }} 
),
{% endif %}

{% if var('sage_intacct__using_invoices', True) %}
ar_invoice as (
    select * 
    from {{ ref('stg_sage_intacct__ar_invoice') }} 
),

ar_invoice_item as (
    select * 
    from {{ ref('stg_sage_intacct__ar_invoice_item') }} 
),
{% endif %}

{% if var('sage_intacct__using_bills', True) %}
ap_bill_enhanced as (
    select
        ap_bill_item.bill_id,
        ap_bill_item.bill_item_id,
        cast(null as {{ dbt.type_string() }}) as invoice_id,
        cast(null as {{ dbt.type_string() }}) as invoice_item_id,
        ap_bill_item.account_no,
        ap_bill_item.account_title,
        ap_bill_item.amount,
        ap_bill_item.class_id,
        ap_bill_item.class_name,
        ap_bill_item.currency,
        ap_bill_item.customer_id,
        ap_bill_item.customer_name,
        ap_bill_item.department_id,
        ap_bill_item.department_name,
        ap_bill_item.entry_date_at,
        ap_bill_item.entry_description,
        ap_bill_item.item_id,
        ap_bill_item.item_name,
        ap_bill_item.line_no,
        ap_bill_item.line_item,
        ap_bill_item.location_id,
        ap_bill_item.location_name,
        ap_bill_item.offset_gl_account_no,
        ap_bill_item.offset_gl_account_title,
        ap_bill_item.total_item_paid,
        ap_bill_item.vendor_id,
        ap_bill_item.vendor_name,
        ap_bill_item.created_at,
        ap_bill_item.modified_at,
        ap_bill.due_in_days,
        ap_bill.total_due,
        ap_bill.total_entered,
        ap_bill.total_paid,
        ap_bill.record_id,
        count(*) over (partition by ap_bill_item.bill_id) as number_of_items
    from ap_bill_item

    left join ap_bill
        on ap_bill_item.bill_id = ap_bill.bill_id
), 
{% endif %}

{% if var('sage_intacct__using_invoices', True) %}
    ar_invoice_enhanced as (
    select 
        cast(null as {{ dbt.type_string() }}) as bill_id,
        cast(null as {{ dbt.type_string() }}) as bill_item_id,
        ar_invoice_item.invoice_id,
        ar_invoice_item.invoice_item_id,
        ar_invoice_item.account_no,
        ar_invoice_item.account_title,
        ar_invoice_item.amount,
        ar_invoice_item.class_id,
        ar_invoice_item.class_name,
        ar_invoice_item.currency,
        ar_invoice_item.customer_id,
        ar_invoice_item.customer_name,
        ar_invoice_item.department_id,
        ar_invoice_item.department_name,
        ar_invoice_item.entry_date_at,
        ar_invoice_item.entry_description,
        ar_invoice_item.item_id,
        ar_invoice_item.item_name,
        ar_invoice_item.line_no,
        ar_invoice_item.line_item,
        ar_invoice_item.location_id,
        ar_invoice_item.location_name,
        ar_invoice_item.offset_gl_account_no,
        ar_invoice_item.offset_gl_account_title,
        ar_invoice_item.total_item_paid,
        ar_invoice_item.vendor_id,
        ar_invoice_item.vendor_name,
        ar_invoice_item.created_at,
        ar_invoice_item.modified_at,
        ar_invoice.due_in_days,
        ar_invoice.total_due,
        ar_invoice.total_entered,
        ar_invoice.total_paid,
        ar_invoice.record_id,
        count(*) over (partition by ar_invoice_item.invoice_id) as number_of_items

        from ar_invoice_item
        left join ar_invoice
            on ar_invoice_item.invoice_id = ar_invoice.invoice_id
    ),
{% endif %}

ap_ar_enhanced as (
    {% if var('sage_intacct__using_bills', True) %}
    select * 
    from ap_bill_enhanced
    {% endif %}

    {% if fivetran_utils.enabled_vars(vars=["sage_intacct__using_bills", "sage_intacct__using_invoices"]) %}
    union all
    {% endif %}

    {% if var('sage_intacct__using_invoices', True) %}
    select * 
    from ar_invoice_enhanced
    {% endif %}

), 

final as (
    select 
        coalesce(
                {% if var('sage_intacct__using_bills', True) %} bill_id {% endif %}

                {% if fivetran_utils.enabled_vars(vars=["sage_intacct__using_bills", "sage_intacct__using_invoices"]) %}
                ,
                {% endif %}

                {% if var('sage_intacct__using_invoices', True) %} invoice_id {% endif %}
        ,null) as document_id,
        coalesce(
                {% if var('sage_intacct__using_bills', True) %} bill_item_id {% endif %}

                {% if fivetran_utils.enabled_vars(vars=["sage_intacct__using_bills", "sage_intacct__using_invoices"]) %}
                , 
                {% endif %}

                {% if var('sage_intacct__using_invoices', True) %}  invoice_item_id {% endif %} 
        ,null) as document_item_id,
        case 
            {% if var('sage_intacct__using_bills', True) %} when bill_id is not null then 'bill' {% endif %} 
            {% if var('sage_intacct__using_invoices', True) %} when invoice_id is not null then 'invoice' {% endif %} 
        end as document_type,
        entry_date_at,
        entry_description,
        amount,
        currency,
        due_in_days,
        item_id,
        item_name,
        line_no,
        line_item, 
        customer_id,
        customer_name,
        department_id,
        department_name,
        location_id,
        location_name,
        vendor_id,
        vendor_name,
        account_no,
        account_title,
        class_id,
        class_name,
        created_at,
        modified_at,
        total_due,
        total_entered,
        total_paid,
        number_of_items,
        total_item_paid,
        offset_gl_account_no,
        offset_gl_account_title,
        record_id
    from ap_ar_enhanced
)

select *
from final
fivetran-joemarkiewicz commented 11 months ago

Thanks for the context @matt-tkachev. While the intention of the packages is that you do not need to fork them, if you have been able to implement this properly elsewhere then I agree that there may be something we need to remedy in the package code.

It seems the code you shared is the dbt version of the code, would you be able to share the compiled version? This code should be in the target folder and will not have any of the dbt-jinja logic within it. It should be the code that is run in your warehouse. Upon inspecting that we should get a good sense of where the error may be happening.

fivetran-joemarkiewicz commented 11 months ago

My apologies @matt-tkachev, I just saw your comment about not being able to access the direct target compile code since it is being orchestrated by Fivetran. You can capture this file if you run the package locally.

Let me see if I can reproduce. In the meantime, do any of the staging models used in this AR and AP model look particularly "off" when you inspect them in your destination?

fivetran-joemarkiewicz commented 11 months ago

@matt-tkachev unfortunately I have not been able to reproduce this error 🤔. If you are unable to find anything that looks off in the underlying data, it may make the most sense to meet over a video call. If you would like, feel free to schedule some time during our office hours. We can then debug this live!

matt-tkachev commented 11 months ago

@fivetran-joemarkiewicz thanks for the information! I didn't know that the initial idea was that we didn't need to fork the original GitHub package repository. I thought that since we had to add an SSH key to the repository so that Fivetran was able to access it, we needed to fork it because we can't add such an access key to the main source repository. So, that was the thought process behind forking the repository and using the fork instead.

Regarding the compiled version, I wasn't able to access it in Fivetran or Snowflake. However, I have found the relevant warehouse error log in Snowflake. Using it and the table definition script for the AP AR Enhanced table, I'm pretty sure I found where the issue lies. After debugging the table definition script, I think that the error appears because of the Class ID column. To be more concrete, in the AP AR Enhanced table definition script, there is an ap_ar_enhanced sub-query that has the following code:

ap_ar_enhanced as (
    select * 
    from ap_bill_enhanced
    union all
    select * 
    from ar_invoice_enhanced
)

Both the ap_bill_enhanced and ar_invoice_enhanced sub-queries that are combined using a union all function have the class_id column. However, ap_bill_enhanced has this column as a varchar from the STG_SAGE_INTACCT__AP_BILL_ITEM table, and ar_invoice_enhanced has it as a number from the STG_SAGE_INTACCT__AR_INVOICE_ITEM table (attached as screenshots). So, I think that the class_id field in the STG_SAGE_INTACCT__AR_INVOICE_ITEM table needs to be converted to a varchar in order for the union all functionality from above to work. I've also checked the final tables for another client that we have, and both the STG_SAGE_INTACCT__AR_INVOICE_ITEM and STG_SAGE_INTACCT__AP_BILL_ITEM tables have the class_id column as a varchar. So, I'm not sure why in this case, STG_SAGE_INTACCT__AR_INVOICE_ITEM defines it as a number. Do you think that I should edit the AP AR Enhanced script to convert the ar_invoice_enhanced class_id field to varchar? STG_AP_BILL_ITEM STG_AR_INVOICE_ITEM

fivetran-rory commented 11 months ago

Hey @fivetran-joemarkiewicz, I am working with Matt on the Support Ticket for this.

I have checked into his latest point regarding the column type differences and confirmed the same in the DBT Docs for the Project.

The [stg_sage_intacct__ar_invoice_item] model (https://fivetran.github.io/dbt_sage_intacct_source/#!/model/model.sage_intacct_source.stg_sage_intacct__ar_invoice_item) defines class_id as an INT:

Screenshot 2023-12-06 at 10 18 56

The stg_sage_intacct__ap_bill_item model defines it as a String:

Screenshot 2023-12-06 at 10 24 55

The Union in the model can be seen at line 122:

ap_ar_enhanced as (
    {% if var('sage_intacct__using_bills', True) %}
    select * 
    from ap_bill_enhanced
    {% endif %}

    {% if fivetran_utils.enabled_vars(vars=["sage_intacct__using_bills", "sage_intacct__using_invoices"]) %}
    union all
    {% endif %}

    {% if var('sage_intacct__using_invoices', True) %}
    select * 
    from ar_invoice_enhanced
    {% endif %}

),
fivetran-joemarkiewicz commented 11 months ago

@matt-tkachev @fivetran-rory thanks so much for sharing the additional information, this has been incredibly helpful!

Upon you raising the class_id as the problem, I believe I unintentionally caught this error last week when I was working on an unrelated update to this package (and the upstream dbt_sage_intacct_source) package. Here is my PR #14 for the source package which includes a safe cast of the class_id to be of type string for destinations. This will ensure the type of error you are seeing does not occur.

I am actually planning on releasing these updates either today or tomorrow, so this is great timing! Another reason it is usually best to install the package versus forking it is you will be able to easily update the package via your packages.yml once I roll these updates out. Otherwise you will need to manually ensure your fork is up to date with the origin. If you are interested in testing the updates in my PR before they go live, you can use the first option in your root packages.yml (or if you would like to still use the fork, you can just update the packages.yml in your fork to the second option).

1.

packages:
  - git: https://github.com/fivetran/dbt_sage_intacct.git
    revision: feature/gl-batch-addition
    warn-unpinned: false 

2.

packages:
  - git: https://github.com/fivetran/dbt_sage_intacct_source.git
    revision: feature/gl-batch-addition
    warn-unpinned: false 

I'll be sure to share updates here once the updates are live on the main branches. Thanks again!

matt-tkachev commented 11 months ago

Hello @fivetran-joemarkiewicz I've tested your PR, and it has solved the issue. The AP AR Enhanced table has been successfully constructed. Thanks for providing it!

As with regards to forking against using the original package, we have added some custom code to add the location columns to the target tables that didn't have them because our clients considered these columns to be essential for their reporting purposes. When we had initially contacted Fivetran Support about the dbt package not having the location columns in some tables, they'd said that they'd had it in their backlog but hadn't specified an ETA for the implementation and had advised us to make our own copy of the repository and develop any needed package additions there. That was the first reason which made us decide to fork the repository and add our custom code for including the location columns there.

Besides the location columns issue, as I've mentioned before, when I was adding the dbt package, there was an instruction in Fivetran Transformations asking to add a public access key for Fivetran to the repository so that Fivetran would have SSH access to the repository. I'm pretty sure that it's only possible to add a public access key to the repositories where you're an owner/admin; thus, I don't believe that it's possible to add a public access key provided by Fivetran to the original/main package repository since obviously I'm not an owner of it. So, that was the second reason behind the forking strategy.

However, since you've mentioned that most organizations add the original package repository to their orchestration platform, my understanding regarding the issues above might be wrong. Please let me know if I can troubleshoot the problems described above without using a custom fork. Thanks for your help once again!

fivetran-joemarkiewicz commented 11 months ago

Hi @matt-tkachev the reasoning for the forking of this repo makes sense to me with your need to include the location field. By the way, if you are interested in contributing that to the main version of the package, we would happily welcome reviewing a PR!

I also wanted to share that the latest update of the dbt_sage_intacct_source package should include the fix to this issue. As such, I will close out this ticket. Thanks again @matt-tkachev and @fivetran-rory for your collaboration efforts on understanding the issue and providing a fix!