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

Add support for partial unique indexes #237

Merged
merged 3 commits into from
Mar 26, 2024
Merged

Conversation

thgreasi
Copy link
Contributor

@thgreasi thgreasi commented Nov 26, 2023

@thgreasi thgreasi changed the title Partial unique index Add support for partial unique indexes Nov 26, 2023
thgreasi added a commit to balena-io/pinejs that referenced this pull request Nov 26, 2023
@thgreasi thgreasi requested a review from Page- November 27, 2023 08:07
thgreasi added a commit to balena-io/pinejs that referenced this pull request Dec 5, 2023
@thgreasi thgreasi marked this pull request as ready for review December 12, 2023 14:58
thgreasi added a commit to balena-io/pinejs that referenced this pull request Jan 5, 2024
@thgreasi thgreasi force-pushed the partial-unique-index branch from 12e3a0c to 555a2f6 Compare January 5, 2024 16:42
@flowzone-app flowzone-app bot enabled auto-merge January 5, 2024 16:43
thgreasi added a commit to balena-io/pinejs that referenced this pull request Feb 6, 2024
thgreasi added a commit to balena-io/pinejs that referenced this pull request Mar 19, 2024
name?: string;
description?: string;
nulls?: string;
predicate?: BooleanTypeNodes;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got the name from the PG CREATE INDEX synopsis.
See: https://www.postgresql.org/docs/16/sql-createindex.html

thgreasi added a commit to balena-io/pinejs that referenced this pull request Mar 19, 2024
src/AbstractSQLCompiler.ts Outdated Show resolved Hide resolved
src/AbstractSQLCompiler.ts Outdated Show resolved Hide resolved
src/AbstractSQLSchemaOptimiser.ts Outdated Show resolved Hide resolved
Comment on lines +39 to +43
const sha = sbvrTypes.SHA.validateSync(
`${tableName}$${JSON.stringify(ruleBody)}`,
).replace(/^\$sha256\$/, '');
// Trim the trigger to a max of 63 characters, reserving at least 32 characters for the hash
return `${tableName.slice(0, 30)}$${sha}`.slice(0, 63);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd make sense for this to take inspiration from the roughly equivalent code in https://github.com/balena-io-modules/odata-to-abstract-sql/blob/v6.2.4/src/odata-to-abstract-sql.ts#L393-L396 - the things of note to me are:

  1. using base36 allows it to still be a string but also fit as much of the hash in as possible in the same number of characters (vs hex/base 16)
  2. adjusting the truncation dynamically based upon the actual strings
  3. using a named constant for MAX_ALIAS_LENGTH rather than a magic number for which the reasoning is not as clear

thgreasi added a commit to balena-io/pinejs that referenced this pull request Mar 21, 2024
@thgreasi thgreasi force-pushed the partial-unique-index branch from 555a2f6 to 2998222 Compare March 21, 2024 14:47
@thgreasi thgreasi requested a review from Page- March 21, 2024 14:47
@thgreasi
Copy link
Contributor Author

It seems that the latest TS fails to compile on master
See: #246

Comment on lines 584 to 587
// Without the `: SbvrType['validate']` widening TS complains that
// "none of those signatures are compatible with each other".
const validateFn: SbvrType['validate'] =
validateTypes[dataType as keyof typeof sbvrTypes];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's on master atm, and will be dropped from this PR once I rebase.

package.json Outdated
@@ -36,7 +36,7 @@
"lint-staged": "^13.2.1",
"mocha": "^10.2.0",
"ts-node": "^10.9.1",
"typescript": "^5.0.4"
"typescript": "^5.4.3"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be dropped once I rebase on top of master

@thgreasi thgreasi force-pushed the partial-unique-index branch from 694b370 to 6d00b7d Compare March 26, 2024 09:25
@flowzone-app flowzone-app bot merged commit 9d857c2 into master Mar 26, 2024
49 checks passed
@flowzone-app flowzone-app bot deleted the partial-unique-index branch March 26, 2024 09:29
thgreasi added a commit to balena-io/pinejs that referenced this pull request Mar 26, 2024
…ption

Update @balena/abstract-sql-compiler from 9.0.4 to 9.1.0

Depends-on: balena-io-modules/abstract-sql-compiler#237
Change-type: minor
thgreasi added a commit to balena-io/pinejs that referenced this pull request Apr 2, 2024
…ption

Update @balena/abstract-sql-compiler from 9.0.4 to 9.1.0

Depends-on: balena-io-modules/abstract-sql-compiler#237
Change-type: minor
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