Skip to content

Feat/default discovery#165

Open
sinder38 wants to merge 3 commits intoSeaQL:masterfrom
sinder38:feat/default-discovery
Open

Feat/default discovery#165
sinder38 wants to merge 3 commits intoSeaQL:masterfrom
sinder38:feat/default-discovery

Conversation

@sinder38
Copy link
Copy Markdown
Contributor

Maybe someday

PR Info

New Features

  • parse pg defaults as defaults instead of extra

Breaking Changes

  • Changes how Postgre schema in parsed

@sinder38
Copy link
Copy Markdown
Contributor Author

The rest of Clippy errors are not mine. Out of scope, so no touching

Also I want to change Skila schema to feature more advanced features, as it stands its too basic.
Should I create a new one or modify Skila?

}
}

pub fn parse_column_default(default: Option<String>) -> Option<ColumnDefault> {
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.

I understand this is intended to keep changes minimal. But this approach quite hacky and hard to maintain. It may be better to refactor ColumnQueryResult retrieval to follow recommended practices. We should also take generated columns into consideration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree with you.
I did it because MySQL parser used the same style:
https://github.com/sinder38/sea-schema/blob/41d89efde457aa5bc70c54609c96c5281c82affd/src/mysql/parser/column.rs#L265

Sqlite one just inlines it.

So if I am changing this it basically mandatory to change MySQL one.

I actually don't like how quite a lot of things are written is sea-*, but pushing for a refactor would require maintainer support and 100% would break the API.

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