Skip to content

Add preliminary Ingres/Vector/Actian Avalanche support#36

Closed
clach04 wants to merge 1 commit intowireservice:masterfrom
clach04:ingres_support
Closed

Add preliminary Ingres/Vector/Actian Avalanche support#36
clach04 wants to merge 1 commit intowireservice:masterfrom
clach04:ingres_support

Conversation

@clach04
Copy link
Copy Markdown

@clach04 clach04 commented Mar 2, 2022

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.

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.
@clach04
Copy link
Copy Markdown
Author

clach04 commented Mar 2, 2022

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:

EDIT 2024-01-09 with permalinks to code:

if isinstance(column.data_type, agate.Text) and dialect == 'mysql':

if isinstance(column.data_type, agate.Number) and dialect in ('mssql', 'mysql', 'oracle'):

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.

Comment on lines +202 to +204
elif dialect == 'ingres' and length > 16000:
# @see https://docs.actian.com/actianx/11.2/index.html#page/SQLRef%2FCharacter_Data_Types.htm%23
# Could be longer but ~16K is a reasonable cut off to switch to CLOB
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preceding condition is "length > 21844" (which is 65,535 bytes divided by 3). The Actian docs mention 32,000 bytes, so not sure how you chose 16,000. I'll go with 10,666 (32,000 divided by 3) https://docs.actian.com/ingres/11.2/index.html#page/SQLRef/Character_Data_Types.htm

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, ~10K is a better safe/sane default. It was a while ago so like you, I cannot figure out how I chose ~16K either 😜

Thanks for taking the time to review and improve the original idea! Happy New Year 🎉

@jpmckinney
Copy link
Copy Markdown
Member

Added in 4f8ad30

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants