ArrayField of Postgres processed incorrectly

[SnakeOilSalesman]

I've found that ArrayField of Postgres is processed incorrectly, which actually does not allow to fetch Array Fields not via ORM, nor SQL Expression Language.
Currently existing tests does not cover this case as they are not fetching not object, not list of values from the table

Tested with:
Python==3.7.5
Django==2.2.5
djangorestframework==3.10.3
aldjemy==0.10.1
sqlalchemy==1.3.16
psycopg2==2.8.3
Postgres==10.5

Looks like here (aldjemy/postgres.py line 16):

    # currently no support for multi-dimensional arrays
    if internal_type in data_types and internal_type != 'ArrayField':
        sub_type = data_types[internal_type](field)
        if not isinstance(sub_type, (list, tuple)):
            sub_type = [sub_type]
    else:
        raise RuntimeError('Unsupported array element type')

author of ArrayField processing copy-pasted similar process from the upper level (aldjemy/table.py line 102):

                if internal_type in DATA_TYPES and hasattr(field, 'column'):
                    typ = DATA_TYPES[internal_type](field)
                    if not isinstance(typ, (list, tuple)):
                        typ = [typ]
                    columns.append(Column(field.column,
                            *typ, primary_key=field.primary_key))

but there is no need to put single field in a list. It will not be unpacked anywhere.

I.e. there is no need in aldjemy/postgres.py to do the following:

        if not isinstance(sub_type, (list, tuple)):
            sub_type = [sub_type]    

Since it will produce not the SA Column instance, but python list in the attribute of SA Table instance.

This will lead to attempt to call method dialect_impl from list instance during fetching, when SQLAlchemy awaits here Column instance.

Internal Server Error: /api/aldjemy-test/
Traceback (most recent call last):
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/rest_framework/views.py", line 505, in dispatch
    response = self.handle_exception(exc)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/rest_framework/views.py", line 465, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/rest_framework/views.py", line 476, in raise_uncaught_exception
    raise exc
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/rest_framework/views.py", line 502, in dispatch
    response = handler(request, *args, **kwargs)
  File "/home/sosam/PycharmProjects/aldjemy-test/flow/views.py", line 2956, in get
    sa1 = AlchemyTest.sa.query().all()
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3246, in all
    return list(self)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3405, in __iter__
    return self._execute_and_instances(context)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3430, in _execute_and_instances
    result = conn.execute(querycontext.statement, self._params)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 984, in execute
    return meth(self, multiparams, params)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/sql/elements.py", line 293, in _execute_on_connection
    return connection._execute_clauseelement(self, multiparams, params)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1103, in _execute_clauseelement
    distilled_params,
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1288, in _execute_context
    e, statement, parameters, cursor, context
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1485, in _handle_dbapi_exception
    util.raise_(exc_info[1], with_traceback=exc_info[2])
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 178, in raise_
    raise exception
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1267, in _execute_context
    result = context.get_result_proxy()
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/dialects/postgresql/psycopg2.py", line 585, in get_result_proxy
    return _result.ResultProxy(self)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/result.py", line 775, in __init__
    self._init_metadata()
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/result.py", line 807, in _init_metadata
    self._metadata = ResultMetaData(self, cursor_description)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/result.py", line 296, in __init__
    textual_ordered,
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/result.py", line 474, in _merge_cursor_description
    for idx, (key, name, obj, type_) in enumerate(result_columns)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/result.py", line 474, in <listcomp>
    for idx, (key, name, obj, type_) in enumerate(result_columns)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 1178, in get_result_processor
    return type_._cached_result_processor(self.dialect, coltype)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/sql/type_api.py", line 512, in _cached_result_processor
    d[coltype] = rp = d["impl"].result_processor(dialect, coltype)
  File "/home/sosam/PycharmProjects/aldjemy-test/venv/lib/python3.7/site-packages/sqlalchemy/dialects/postgresql/array.py", line 333, in result_processor
    item_proc = self.item_type.dialect_impl(dialect).result_processor(
AttributeError: 'list' object has no attribute 'dialect_impl'

Removal of those two lines of code fixed the issue. I've checked it with ORM and SQL Expression Language.
Walked through with debugger and compared the resulting column type. It's the same as of Table build with declarative_base.

Resulting column data types are valid:

I can make it a PR with few additional tests.

[ryanhiebert]

A pull request would be great. In particular, I'll be looking for tests of the existing working array functionality, to make sure those don't break. Confirming that they already exist sufficiently is fine. I'm certain that I do have an ArrayField in my production work project that is working, so as long as I don't break that, I'm happy to clean up other issues.

It seems like you've got a reasonable grasp on this issue. I don't yet. But a pull request would be welcome, and I will review it. Thank you for reporting the issue.

[SnakeOilSalesman]

While preparing a branch for this PR I was running tests with tox and found few more issues.

1. It seems that latest python3 versions that is used by tox by default having sqlite3 versions incompatible with specified in tox.ini Django versions 2.0.9 and 2.1.3. I had a similar error as described here:
   https://stackoverflow.com/questions/53637182/django-no-such-table-main-auth-user-old

Updating Django version to 2.0.10 and 2.1.5 fixed the issue.

2. In postgres test project there is a model named JsonModel. But no migration to add it to DB provided. Every test run fails for me with attempt to fetch JsonModel table right after creation of test database.

Adding migration for JsonModel fixed this as well.

Should I create separate PRs for this two?

[ryanhiebert]

1. I'm not sure how you hit that issue, unless you just had an old .tox directory somehow. The tox.ini config should be using the latest patch releases of Django.
2. If the change is related to this change, or uncovered by your new work, go ahead and make it the same pull request, and explain what happened and why your change uncovered it. If it already exists in the master branch, then I'd make it a separate issue, so that it's easier for me to review and merge it separately.

[SnakeOilSalesman]

1. I'm not sure how you hit that issue, unless you just had an old .tox directory somehow. The tox.ini config should be using the latest patch releases of Django.

Gosh, my bad. I've missed that update. Sorry about that.

2. If the change is related to this change, or uncovered by your new work, go ahead and make it the same pull request, and explain what happened and why your change uncovered it. If it already exists in the master branch, then I'd make it a separate issue, so that it's easier for me to review and merge it separately.

it's in the master branch now. Looks like a separate case to me. I'll provide more info later.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ryanhiebert]

I believe this is now fixed in #101 .