clach04
commented
2 years ago Thanks for making this and https://github.com/wireservice/csvkit available. I just used csvkit for the first time this week and whilst I was already using a similar tool, csvkit is significantly more useful and robust 👍
I opened this PR mostly to start discussion, although if it's approved that would be nice too ;-)
What do you think about reversing the logic for the string length code and also the decimal precision/scale code so that they always fire irrespective of the dialect? I.e. the lines with dialect checks:
- https://github.com/wireservice/agate-sql/blob/master/agatesql/table.py#L197
- https://github.com/wireservice/agate-sql/blob/master/agatesql/table.py#L209
EDIT 2024-01-09 with permalinks to code: https://github.com/wireservice/agate-sql/blob/e0170dbb47958d9d3386d72b5c3ba094d2cc16ce/agatesql/table.py#L197 https://github.com/wireservice/agate-sql/blob/e0170dbb47958d9d3386d72b5c3ba094d2cc16ce/agatesql/table.py#L209
Either remove the conditional check OR replace with an alternative check if there are some backends which should not apply this logic. Which backends should this NOT be done for? sqlite doesn't need it from a DDL perspective (as sqlite essentially ignores the type spec) but I'm not sure about the SQLAlchemy dialect. I've not tested this out, I've only tested the code in the PR hence the question and the conservative code change.
This works great when used with https://github.com/clach04/ingres_sa_dialect which is a (perpetually) in-progress Ingres dialect for SQLAlchemy.
jpmckinney
Ingres, like most DBMS implementation, DDL should contain the exact length/precision and scale lengths. The defaults if omitted work but are not likely to be desired.