Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlite exporter #446

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bconnorwhite
Copy link

Summary

  • Added sqlite exporter

Issue

#14
#286

Lasting Changes (Technical)

  • Added sqlite exporter

Checklist

Please check directly on the box once each of these are done

  • Documentation (if necessary)
  • Tests (integration test/unit test)
  • Integration Tests Passed
  • Code Review

@bconnorwhite
Copy link
Author

I didn't add an importer and only a simple test, but this worked for my usecase

Copy link
Contributor

@nguyenalter nguyenalter left a comment

Choose a reason for hiding this comment

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

Please implement the code to export many-to-many relationships in DBML.
You can implement it similar to this function:

static buildForeignKeyManyToMany (fieldsMap, foreignEndpointFields, refEndpointTableName, foreignEndpointTableName, refEndpointSchema, foreignEndpointSchema, model) {

@@ -30,6 +31,10 @@ class ModelExporter {
res = SqlServerExporter.export(normalizedModel);
break;

case 'sqlite':
res = SqliteExporter.export(normalizedModel);
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be best if you could use the same class name SQLiteExporter as the class definition to make it easy to debug in the future.

Comment on lines +90 to +92
static exportRefs (refIds, model, usedTableNames) {
return [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I found that SQLite does not support the ALTER TABLE ... ADD FOREIGN KEY ... syntax (see: https://www.sqlite.org/omitted.html).

I believe that we can move the relationship definitions in the CREATE TABLE ... statement.

Comment on lines +8 to +12
Table "order_items" {
"order_id" integer
"product_id" integer
"quantity" integer [default: 1]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Although the DBML syntax is correct, this mapping table should be defined after the products table so that when we export to SQLite, if the foreign key constraints are added in the CREATE TABLE "ORDER_ITEMS" ... statement, the exported SQL is still correct.

Comment on lines +94 to +96
static exportIndexes (indexIds, model) {
return [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please export the indexes.

Comment on lines +64 to +66
static getTableContentArr (tableIds, model) {
const tableContentArr = tableIds.map((tableId) => {
const fieldContents = SQLiteExporter.getFieldLines(tableId, model);
Copy link
Contributor

Choose a reason for hiding this comment

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

This function needs to export the composite primary keys. You can refer to

const compositePKs = PostgresExporter.getCompositePKs(tableId, model);
to implement it

@Mte90
Copy link

Mte90 commented Nov 7, 2023

Any updates for this?

@aaronlippold
Copy link

Ping

@galvez
Copy link

galvez commented Mar 24, 2024

Hey @nguyenalter I'm picking this up — just checking in to see if you're still around to review and merge when done.

@nguyenalter
Copy link
Contributor

@galvez Thank you for your help.

Feel free to create another PR. We will be there to review it when everything is ready.

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.

5 participants