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

Support object with schema and table name in more places #105

Merged
merged 3 commits into from
Aug 28, 2017

Conversation

aschrab
Copy link
Contributor

@aschrab aschrab commented Aug 25, 2017

  • Allow using an object with schema and table name as value for the references key to setup a foreign key constraint.
  • If an object with schema and table name is passed to createIndex don't include the schema when generating a name for the index.
  • When generating a constraint name for a composite primary key get the table name out of an object if necessary to avoid including [object Object] in the name.

Copy link
Contributor

@dolezel dolezel left a comment

Choose a reason for hiding this comment

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

👍
Only one minor comment...

@@ -35,7 +37,7 @@ function parseColumns(columns, table_name, extending_type_shorthands = {}) {
constraints.push(`CHECK (${options.check})`);
}
if (options.references) {
constraints.push(`REFERENCES ${options.references}`);
constraints.push(`REFERENCES "${schemalize(options.references)}"`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would write it as

template`REFERENCES "${options.references}"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I hadn't noticed that the template function used schemalize. I've made the requested change.

When creating an index on a table where the schema is specified, the
schema shouldn't be included in the index name.
@dolezel dolezel merged commit 85bb309 into salsita:master Aug 28, 2017
@aschrab aschrab deleted the schema branch September 9, 2017 16:37
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