While working on an SQL target I am getting this error

ValueError: Unable to merge sql types: INTEGER, INTEGER

during a

meltano run

. The error is coming from

File "C:\development\sdk\singer_sdk\streams\sql.py", line 744, in merge_sql_types

. I was wondering if

merge_sql_types

should be working with

integer

types or does it only work with

string

and

unicode

types at the moment?

@visch ran into this also.

I do have a fix for this one, was working through another portion of the code

Sidebar: you two both got shoutouts in our release party video today 🙂 thankyou

In merge sql types add this

if str(sql_types[0]) == str(sql_types[1]):
    return sql_types[0]

Whole function looks like this right now

def merge_sql_types(
        self, sql_types: List[sqlalchemy.types.TypeEngine]
    ) -> sqlalchemy.types.TypeEngine:
        """Return a compatible SQL type for the selected type list.

        Args:
            sql_types: List of SQL types.

        Returns:
            A SQL type that is compatible with the input types.

        Raises:
            ValueError: If sql_types argument has zero members.
        """
        if not sql_types:
            raise ValueError("Expected at least one member in `sql_types` argument.")

        if len(sql_types) == 1:
            return sql_types[0]

        sql_types = self._sort_types(sql_types)

        if len(sql_types) > 2:
            return self.merge_sql_types(
                [self.merge_sql_types([sql_types[0], sql_types[1]])] + sql_types[2:]
            )

        assert len(sql_types) == 2

        # SQL Alchemy overrides column type comparisons (only difference
        # between super is these 2 lines)
        if str(sql_types[0]) == str(sql_types[1]):
            return sql_types[0]

        generic_type = type(sql_types[0].as_generic())
        if isinstance(generic_type, type):
            if issubclass(
                generic_type,
                (sqlalchemy.types.String, sqlalchemy.types.Unicode),
            ):
                return sql_types[0]

        elif isinstance(
            generic_type,
            (sqlalchemy.types.String, sqlalchemy.types.Unicode),
        ):
            return sql_types[0]

        raise ValueError(
            f"Unable to merge sql types: {', '.join([str(t) for t in sql_types])}"
        )

still working through some other stuff so I haven't pushed this

Sweet thanks for the updated code. I am still lost on the

if isisntance

and

elif instance

almost seemed like they should have been combined.

if isinstance(generic_type, type):
            if issubclass(
                generic_type,
                (sqlalchemy.types.String, sqlalchemy.types.Unicode),
            ):
                return sql_types[0]
            elif isinstance(
                generic_type,
                (sqlalchemy.types.String, sqlalchemy.types.Unicode),
            ):
                return sql_types[0]

if issubclass(

I think the idea is to check if the type is compatible with the other type using subclasses.

@thomas_briggs you're talking about why the

if isinstance(generic_type, type):
            if issubclass(
                generic_type,
                (sqlalchemy.types.String, sqlalchemy.types.Unicode),
            ):
                return sql_types[0]

check doesn't' work? It's because it's only checking Strings/Unicode types! There wasn't a check added for non Strings

Yeah that could be too 😛 It's been a while since I reported that so my recollection of what I found while investigating it is definitely suspect. 😉

@visch Here is what is bothering me. I interpret the function to be setup to use recursion to get down to one final item in the list that is validated against sqlalchemy

generic, string, and unicode

. I think the way the if statements are setup are not allowing the recursion for

generic

types to complete. The

assert

alerts us to the final conflict. The if statement you and Thomas added allows you to get past the error by not allowing the final check against

sqltype[0]

and

sqltype[1]

if they are equal.

@BuzzCutNorman and @visch - thanks both for taking a look at this. Admittedly this area of the code is not well polished, and we are totally open to rewriting/refactoring as needed. Not having a ton of SQL targets in the wild as of yet on the SDK is a bit of a curse (in that we don't have many successful examples) but it's also a blessing because we can change what we need to change in order to improve things. (Goes without saying, but PRs are welcome and much appreciated.)

This has been merged! https://github.com/meltano/sdk/pull/898