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]: Add sql-datatype to the SDK discovery and catalog. #1323

Open
BuzzCutNorman opened this issue Jan 13, 2023 · 4 comments · May be fixed by #1872
Open

[Feature]: Add sql-datatype to the SDK discovery and catalog. #1323

BuzzCutNorman opened this issue Jan 13, 2023 · 4 comments · May be fixed by #1872
Labels
Accepting Pull Requests kind/Feature New feature or request SQL Support for SQL taps and targets valuestream/SDK

Comments

@BuzzCutNorman
Copy link
Contributor

Feature scope

Taps (catalog, state, stream maps, etc.)

Description

It would be very helpful if SDK based taps during the discovery process of a database grabbed column data types and placed them into the catalog metadata area for the column in the field of sql-datatype. The sql-datatype field is a reserved keyword mentioned in the singer discovery documentation.

https://github.com/singer-io/getting-started/blob/master/docs/DISCOVERY_MODE.md#metadata

Keyword Tap Type Discoverable? Description
sql-datatype database discoverable Represents the datatype of the database column
        {
          "breadcrumb": [
            "properties",
            "name"
          ],
          "metadata": {
            "sql-datatype": "varchar(255)",
            "selected-by-default": true
          }
        }
@BuzzCutNorman BuzzCutNorman changed the title [Feature]: Added sql-datatype to the SDK discovery and catalog [Feature]: Add sql-datatype to the SDK discovery and catalog. Jan 13, 2023
@stale
Copy link

stale bot commented Jul 18, 2023

This has been marked as stale because it is unassigned, and has not had recent activity. It will be closed after 21 days if no further activity occurs. If this should never go stale, please add the evergreen label, or request that it be added.

@edgarrmondragon
Copy link
Collaborator

Related: #1903

@edgarrmondragon
Copy link
Collaborator

Copying here my comment from #1872 (comment):

One thing that unfortunately isn't very clear to me is how are targets supposed to consume this information since a target isn't aware of the tap's metadata. It might be better if this metadata lived in the schema, at least that way it would be emitted with SCHEMA messages and could be used by the target.

The official Singer docs only explain the field as Represents the datatype of a database column, and pipelinewise's taps implement but the target don't seem to use it in any way:

https://github.com/search?q=org%3Atransferwise+sql-datatype+language%3APython&type=code&l=Python

Otherwise, who is the consumer of this metadata?

@BuzzCutNorman
Copy link
Contributor Author

BuzzCutNorman commented Dec 2, 2024

I am guessing the consumer might be the Stich platform. During this conversation I had in the Meltano Slack:

https://discuss.meltano.com/t/16288098/i-was-wondering-if-i-someone-could-point-me-in-the-proper-di

We came to the same conclusion that this would need to be a part of the SCHEMA message to become useful. The alternative of using JSON Shema attributes to describe enough of the specifics to determine what the types are was mentioned. This has been done in countless other taps and targets just there is not a standardization of it.

knowing the database type (posgresql, sql server, oracle) and sql data type ( integer(), varchar(10) ) would be easier and maybe more accurate than the JSON Schema fingerprint method.

I find myself looking back on this and almost thinking this feature request should be closed. Please let me know wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepting Pull Requests kind/Feature New feature or request SQL Support for SQL taps and targets valuestream/SDK
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants