Skip to content

Fix issue with Postgres 10#138

Merged
simonw merged 3 commits intosimonw:mainfrom
ryancheley:main
Jul 4, 2021
Merged

Fix issue with Postgres 10#138
simonw merged 3 commits intosimonw:mainfrom
ryancheley:main

Conversation

@ryancheley
Copy link
Copy Markdown
Contributor

@ryancheley ryancheley commented Jul 4, 2021

When using version 1.0 on Ubuntu 18.04 with Postgres 10.17 an Internal Server (500) error is generated.

I traced this back to the change made for version 1.0 on line 162 (see compared diff here)

The array_agg will throw the error

could not find array type for data type information_schema.sql_identifier

casting the column_name field to text resolves this issue for Postgres 10.

I did run the tests locally and 88 passed with 2 warnings. The warnings were:

test_project/test_utils.py::test_apply_sort[select * from (select * from foo) as results order by "bar"-bar-True-select * from (select * from foo) as results order by "bar" desc]
  /Users/ryan/github/django-sql-dashboard/venv/lib/python3.9/site-packages/django/db/backends/postgresql/base.py:304: RuntimeWarning: Normally Django will use a connection to the 'postgres' database to avoid running initialization queries against the production database when it's not needed (for example, when running tests). Django was unable to create a connection to the 'postgres' database and will use the first PostgreSQL database instead.
    warnings.warn(

test_project/test_utils.py::test_apply_sort[select * from (select * from foo) as results order by "bar"-bar-True-select * from (select * from foo) as results order by "bar" desc]
  /Users/ryan/github/django-sql-dashboard:0: PytestWarning: Error when trying to teardown test databases: RuntimeError("generator didn't stop after throw()")

-- Docs: https://docs.pytest.org/en/stable/warnings.html

ryancheley added 3 commits July 4, 2021 12:16
@ryancheley
Fixed issue with Postgres 10
a545abb 
When using Postgres 10 the `information_schema.columns.column_name` needs to be cast as text. Otherwise, an error is generated which states:

```
could not find array type for data type information_schema.sql_identifier
```
@ryancheley
Merge pull request #1 from ryancheley/ryancheley-patch-1
248a6e9 
Fixed issue with Postgres 10
@ryancheley
Fixed typo
23d672c 
There was a missing space between the -m and venv that could lead to confusion for new contributors
@simonw
Copy link
Copy Markdown
Owner

simonw commented Jul 4, 2021

Thanks for figuring this out!

@simonw
Copy link
Copy Markdown
Owner

simonw commented Jul 4, 2021

It would be great to have the GitHub Actions workflow run tests against multiple PostgreSQL versions to catch this kind of thing in the future https://github.com/simonw/django-sql-dashboard/blob/main/.github/workflows/test.yml

@simonw simonw merged commit 65e7d51 into simonw:main Jul 4, 2021
@simonw simonw mentioned this pull request Jul 4, 2021
Closed
@ryancheley
Copy link
Copy Markdown
Contributor Author

ryancheley commented Jul 4, 2021

@simonw no problem! Sorry if the commit history looks wonky. As I was working through the contribution guidelines I found a typo which I fixed, but had messed up all of the branches on my local version and was trying to undo and redo and ... well, it's a holiday Sunday and I think it's time I go enjoy time with my family! Cheers!

simonw added a commit that referenced this pull request Jul 6, 2021
@simonw
Revert "Fix issue with Postgres 10 (#138)" - to help test #139
9bc6ff2 
This reverts commit 65e7d51.
simonw pushed a commit that referenced this pull request Jul 6, 2021
@ryancheley @simonw
Fix issue with Postgres 10 (#138)
c3f7bee 
* Fixed issue with Postgres 10

When using Postgres 10 the `information_schema.columns.column_name` needs to be cast as text. Otherwise, an error is generated which states:

```
could not find array type for data type information_schema.sql_identifier
```

* Fixed typo

There was a missing space between the -m and venv that could lead to confusion for new contributors
@simonw simonw mentioned this pull request Jul 6, 2021
Merged
simonw added a commit that referenced this pull request Jul 6, 2021
@simonw
Release 1.0.1
a5fc78f 
Refs #137, #138, #139
simonw added a commit that referenced this pull request Jul 6, 2021
@simonw
Release 1.0.1
ea3c2be 
Refs #137, #138, #139
@simonw
Copy link
Copy Markdown
Owner

simonw commented Aug 9, 2021

Ryan wrote about this fix here: https://www.ryancheley.com/2021/07/09/contributing-to-django-sql-dashboard/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ryancheley @simonw