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

[CT-2043] [Bug] Missing type (double) in is_float for databricks #6876

Closed
2 tasks done
rlh1994 opened this issue Feb 6, 2023 · 2 comments · Fixed by #6897
Closed
2 tasks done

[CT-2043] [Bug] Missing type (double) in is_float for databricks #6876

rlh1994 opened this issue Feb 6, 2023 · 2 comments · Fixed by #6897
Labels
bug Something isn't working Team:Adapters Issues designated for the adapter area of the code

Comments

@rlh1994
Copy link
Contributor

rlh1994 commented Feb 6, 2023

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

I'm not sure if this belongs in here or the spark or the databricks adapator, but a double type column in databricks returns false for all of is_float, is_numeric, and is_number.

Expected Behavior

It would return true for all those class method.

Steps To Reproduce

  1. Create a databricks table with a double column
  2. Use adapter.get_columns_in_relation(your_table) and loop over the columns, logging or adding comments for the .data_type and the .is_number()
  3. Observe the column with type double has false for is_number()

Relevant log output

No response

Environment

- OS:
- Python:
- dbt: 1.4.1

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

Using databricks as unable to test on spark, it is a delta table if that is relevant.

I believe it just needs to be added to the list here:

def is_float(self):
return self.dtype.lower() in [
# floats
"real",
"float4",
"float",
"double precision",
"float8",
]

@rlh1994 rlh1994 added bug Something isn't working triage labels Feb 6, 2023
@github-actions github-actions bot changed the title [Bug] Missing type (double) in is_float for databricks [CT-2043] [Bug] Missing type (double) in is_float for databricks Feb 6, 2023
@jtcohen6
Copy link
Contributor

jtcohen6 commented Feb 7, 2023

Thanks for opening @rlh1994!

While both dbt-spark and dbt-databricks implement their own Column class (SparkColumn and DatabricksColumn), it doesn't look like either reimplements the is_float method. So, they're just using the BaseAdapter.Column method, defined in dbt-core, which you linked above.

Given that we already have double precision in the list above, and other data platforms (e.g. Snowflake) also treat DOUBLE and DOUBLE PRECISION as true aliases, it seems reasonable to me that we'd add double to the default is_float method.

Is that a change you'd be up for making?

@jtcohen6 jtcohen6 added Team:Adapters Issues designated for the adapter area of the code and removed triage labels Feb 7, 2023
@rlh1994
Copy link
Contributor Author

rlh1994 commented Feb 7, 2023

@jtcohen6 Sure I'll get a PR in for later, I can't immediately find any tests for this method anyway but we can discuss that in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Team:Adapters Issues designated for the adapter area of the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants