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

UI changes for check constraint support #979

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

taherkl
Copy link
Contributor

@taherkl taherkl commented Dec 30, 2024

Users can view details of check constraints in the Check Constraint tab.
Users will be able to add, delete, and edit check constraints.
Deletion of a column is restricted if it is used in a check constraint.
Validation is performed to prevent adding duplicate expressions or constraint names in check constraints.
Check constraints have been incorporated into the Spanner DDL query.
Integration with Verify Expression API

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to README are included in PR

@asthamohta
Copy link
Collaborator

Please checkout package json file

} else if (this.checkIfCcColumn(colId)) {
let message = `Column ${spColName} is a part of`;
const dependencies = [];
if (this.checkIfPkColumn(colId)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

code will never enter this block

Copy link
Contributor

Choose a reason for hiding this comment

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

comment is addressed


this.ccArray.value.forEach((cc: ICcTabData) => {
if (cc.spConstraintName && cc.spConstraintCondition) {
if (spCkArr.some(item => item.Name === cc.spConstraintName || item.Expr === cc.spConstraintCondition)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For two check constraints that are empty and have no name, this will throw an error, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

it will not throw error because of the constraint without name is also acceptable
Screenshot 2025-01-02 at 4 16 39 PM

akashthawaitcc and others added 3 commits January 2, 2025 11:37
* remove the block which will not execute

* refactor the code
1. remove the unreachable code
2.  remvoe the node-sql-parser package

* refactor the code
1. rename the generateId method

---------

Co-authored-by: Vivek Yadav <vivek.yadav@ollion.com>
@taherkl taherkl marked this pull request as ready for review January 3, 2025 04:49
@taherkl taherkl requested a review from a team as a code owner January 3, 2025 04:49
@taherkl taherkl requested review from darshan-sj and Deep1998 and removed request for a team January 3, 2025 04:49
@@ -1,7 +1,7 @@
{
"name": "ui",
"version": "0.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are there changes in this file ? Is it intended?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes we have removed the node-sql-parse package

Copy link
Collaborator

Choose a reason for hiding this comment

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

in that case why is there no change in package.json file? From where was this package removed

Copy link
Contributor

Choose a reason for hiding this comment

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

updated the package.lock file, no there are no changes

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please run ng build and check in the updated files matching index.html in ui/dist

Copy link
Contributor

Choose a reason for hiding this comment

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

sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants