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

Feature suggestions: Infer DECIMAL type from multipleOf #22

Closed
paultiplady opened this issue Oct 26, 2021 · 5 comments · Fixed by #23
Closed

Feature suggestions: Infer DECIMAL type from multipleOf #22

paultiplady opened this issue Oct 26, 2021 · 5 comments · Fixed by #23
Assignees

Comments

@paultiplady
Copy link

Currently there is some support for bq-decimal types, but it requires overriding the schema to use non-standard types.

#11

Looking at the output from tap-mysql, it seems that we could unambiguously identify NUMERIC / DECIMAL (same thing in BQ) by looking at the full schema entry:

          "amount": {
            "inclusion": "available",
            "multipleOf": 0.01,
            "type": [
              "null",
              "number"
            ]
          },

It seems that if we have a type=number that also specifies multipleOf, then you're dealing with a DECIMAL not a float.

The above came from a `amount` numeric(65, 2) NOT NULL column, so it seems that tap-mysql doesn't correctly propagate the digits part, which could in principle be used to infer DECIMAL vs. BIGDECIMAL in the BigQuery schema. But for small decimals perhaps it's possible to automatically infer the type more accurately.

(I don't know if the multipleOf schema is used more widely.)

@RuslanBergenov
Copy link
Collaborator

Sorry for late reply @paultiplady. We will discuss this and get back to you.

@paultiplady
Copy link
Author

No rush! We have a fork that hardcodes a non-generic solution to this issue but would rather be following your repo.

@RuslanBergenov
Copy link
Collaborator

@paultiplady, we are okay with this change, thank you for your contribution. I merged your Pull Request into a dev branch. We'll test it more, make some tweaks, and do a code review, before we release it.

cc @zvizdo

@RuslanBergenov
Copy link
Collaborator

RuslanBergenov commented Dec 6, 2021

@paultiplady, we have made some changes to the feature. This branch is currently going through a code review.

@RuslanBergenov
Copy link
Collaborator

Thank you for your contribution @paultiplady, we have just released your feature.

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 a pull request may close this issue.

2 participants